Merge pull request #2293 from hypothesis/typecheck-svg-icon

Typecheck SvgIcon

src/shared/components/svg-icon.js

@@ -6,7 +6,12 @@ import propTypes from 'prop-types';
 /**
  * Object mapping icon names to SVG markup.
  *
- * @typedef {{[name: string]: string}} IconMap
+ * @typedef {Object.<string,string>} IconMap
+ */
+
+/**
+ * @template T
+ * @typedef {import("preact/hooks").Ref<T>} Ref
  */
 
 /**
@@ -16,12 +21,24 @@ import propTypes from 'prop-types';
  */
 let iconRegistry = {};
 
+/**
+ * @typedef SvgIconProps
+ * @prop {string} name - The name of the icon to display.
+ *   The name must match a name that has already been registered using the
+ *   `registerIcons` function.
+ * @prop {string} [className] - A CSS class to apply to the `<svg>` element.
+ * @prop {boolean} [inline] - Apply a style allowing for inline display of icon wrapper.
+ * @prop {string} [title] - Optional title attribute to apply to the SVG's containing `span`.
+ */
+
 /**
  * Component that renders icons using inline `<svg>` elements.
  * This enables their appearance to be customized via CSS.
  *
  * This matches the way we do icons on the website, see
  * https://github.com/hypothesis/h/pull/3675
+ *
+ * @param {SvgIconProps} props
  */
 export default function SvgIcon({
   name,
@@ -34,10 +51,15 @@ export default function SvgIcon({
   }
   const markup = { __html: iconRegistry[name] };
 
-  const element = useRef();
+  const element = /** @type {Ref<HTMLElement>} */ (useRef());
   useLayoutEffect(() => {
     const svg = element.current.querySelector('svg');
-    svg.setAttribute('class', className);
+
+    // The icon should always contain an `<svg>` element, but check here as we
+    // don't validate the markup when it is registered.
+    if (svg) {
+      svg.setAttribute('class', className);
+    }
   }, [
     className,
     // `markup` is a dependency of this effect because the SVG is replaced if
@@ -61,29 +83,18 @@ export default function SvgIcon({
 }
 
 SvgIcon.propTypes = {
-  /**
-   * The name of the icon to display.
-   *
-   * The name must match a name that has already been registered using the
-   * `registerIcons` function.
-   */
-  name: propTypes.string,
-
-  /** A CSS class to apply to the `<svg>` element. */
+  name: propTypes.string.isRequired,
   className: propTypes.string,
-
-  /** Apply a style allowing for inline display of icon wrapper */
   inline: propTypes.bool,
-
-  /** Optional title attribute to apply to the SVG's containing `span` */
   title: propTypes.string,
 };
 
 /**
  * Register icons for use with the `SvgIcon` component.
  *
- * @param {IconMap} icons - Object mapping icon names to SVG data.
- * @param {boolean} [options.reset] - If `true`, remove existing registered icons.
+ * @param {IconMap} icons
+ * @param {Object} options
+ *  @param {boolean} [options.reset] - If `true`, remove existing registered icons.
  */
 export function registerIcons(icons, { reset = false } = {}) {
   if (reset) {

src/tsconfig.json

@@ -15,6 +15,7 @@
     "annotator/util/*.js",
     "boot/*.js",
     "shared/*.js",
+    "shared/components/*.js",
     "sidebar/*.js",
     "sidebar/services/*.js",
     "sidebar/util/*.js"