more code review

Author: avivkeller

src/generators/jsx/constants.mjs

@@ -1,19 +1,34 @@
-// Maps Node.js API stability indices (0-3) to UI component stability levels.
+/**
+ * UI classes for Node.js API stability levels
+ *
+ * @see https://nodejs.org/api/documentation.html#stability-index
+ */
 export const STABILITY_LEVELS = [
   'danger', // (0) Deprecated
   'warning', // (1) Experimental
   'success', // (2) Stable
   'info', // (3) Legacy
 ];
 
-// Maps HTML tags to corresponding component names in @node-core/ui-components.
+/**
+ * HTML tag to UI component mappings
+ */
 export const TAG_TRANSFORMS = {
   pre: 'CodeBox',
   blockquote: 'Blockquote',
 };
 
-// Maps API heading types to their CircularIcon props.
-export const ICON_SYMBOL_MAP = {
+/**
+ * @see transformer.mjs's TODO comment
+ */
+export const TYPE_TRANSFORMS = {
+  raw: 'text',
+};
+
+/**
+ * API type icon configurations
+ */
+export const API_ICONS = {
   event: { symbol: 'E', color: 'red' },
   method: { symbol: 'M', color: 'red' },
   property: { symbol: 'P', color: 'red' },
@@ -23,29 +38,78 @@ export const ICON_SYMBOL_MAP = {
   ctor: { symbol: 'C', color: 'red' },
 };
 
-// Maps API lifecycle change type identifiers to their human-readable labels.
-export const CHANGE_TYPES = {
+/**
+ * API lifecycle change labels
+ */
+export const LIFECYCLE_LABELS = {
   added_in: 'Added in',
   deprecated_in: 'Deprecated in',
   removed_in: 'Removed in',
   introduced_in: 'Introduced in',
 };
 
-export const AST_NODES = {
+// TODO(@avivkeller): These should be inherited from @node-core/website-i18n
+export const INTERNATIONALIZABLE = {
+  sourceCode: 'Source Code: ',
+};
+
+/**
+ * Abstract Syntax Tree node type constants
+ */
+export const AST_NODE_TYPES = {
   MDX: {
-    // https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxtextelement
+    /**
+     * Text-level JSX element
+     *
+     * @see https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxtextelement
+     */
     JSX_INLINE_ELEMENT: 'mdxJsxTextElement',
-    // https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxflowelement
+
+    /**
+     * Block-level JSX element
+     *
+     * @see https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxflowelement
+     */
     JSX_BLOCK_ELEMENT: 'mdxJsxFlowElement',
-    // https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxattribute
+
+    /**
+     * JSX attribute
+     *
+     * @see https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxattribute
+     */
     JSX_ATTRIBUTE: 'mdxJsxAttribute',
-    // https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxattributevalueexpression
+
+    /**
+     * JSX expression attribute
+     *
+     * @see https://github.com/syntax-tree/mdast-util-mdx-jsx#mdxjsxattributevalueexpression
+     */
     JSX_ATTRIBUTE_EXPRESSION: 'mdxJsxAttributeValueExpression',
   },
   ESTREE: {
-    // https://github.com/estree/estree/blob/master/es5.md#programs
+    /**
+     * AST Program node
+     *
+     * @see https://github.com/estree/estree/blob/master/es5.md#programs
+     */
     PROGRAM: 'Program',
-    // https://github.com/estree/estree/blob/master/es5.md#expressionstatement
+
+    /**
+     * Expression statement
+     *
+     * @see https://github.com/estree/estree/blob/master/es5.md#expressionstatement
+     */
     EXPRESSION_STATEMENT: 'ExpressionStatement',
   },
+  // TODO(@avivkeller): These should be inherited from the elements themselves
+  JSX: {
+    ALERT_BOX: 'AlertBox',
+    CHANGE_HISTORY: 'ChangeHistory',
+    CIRCULAR_ICON: 'CircularIcon',
+    NAV_BAR: 'NavBar',
+    ARTICLE: 'Article',
+    SIDE_BAR: 'SideBar',
+    META_BAR: 'MetaBar',
+    FOOTER: 'Footer',
+  },
 };

src/generators/jsx/index.mjs

@@ -24,7 +24,7 @@ export default {
    *
    * @param {Input} entries
    * @param {Partial<GeneratorOptions>} options
-   * @returns {Promise<string[]>} Array of generated content
+   * @returns {Promise<Array<string>>} Array of generated content
    */
   async generate(entries, { releases, version }) {
     const remarkRecma = getRemarkRecma();

src/generators/jsx/utils/ast.mjs

@@ -2,17 +2,19 @@
 
 import { u as createTree } from 'unist-builder';
 import { valueToEstree } from 'estree-util-value-to-estree';
-import { AST_NODES } from '../constants.mjs';
+import { AST_NODE_TYPES } from '../constants.mjs';
+
+/**
+ * @typedef {Object} JSXOptions
+ * @property {boolean} [inline] - Whether the element is inline
+ * @property {(string | Array<import('unist').Node>)} [children] - Child content or nodes
+ */
 
 /**
  * Creates an MDX JSX element with support for complex attribute values.
  *
  * @param {string} name - The name of the JSX element
- * @param {{
- * inline?: boolean,
- * children?: string | import('unist').Node[],
- * [key: string]: any
- * }} [options={}] - Options including type, children, and JSX attributes
+ * @param {JSXOptions & Record<string, any>} [options={}] - Options including type, children, and JSX attributes
  * @returns {import('unist').Node} The created MDX JSX element node
  */
 export const createJSXElement = (
@@ -26,8 +28,8 @@ export const createJSXElement = (
       : children;
 
   const elementType = inline
-    ? AST_NODES.MDX.JSX_INLINE_ELEMENT
-    : AST_NODES.MDX.JSX_BLOCK_ELEMENT;
+    ? AST_NODE_TYPES.MDX.JSX_INLINE_ELEMENT
+    : AST_NODE_TYPES.MDX.JSX_BLOCK_ELEMENT;
 
   const attrs = Object.entries(attributes).map(([key, value]) =>
     createAttributeNode(key, value)
@@ -50,15 +52,15 @@ export const createJSXElement = (
 function createAttributeNode(name, value) {
   // Use expression for objects and arrays
   if (value !== null && typeof value === 'object') {
-    return createTree(AST_NODES.MDX.JSX_ATTRIBUTE, {
+    return createTree(AST_NODE_TYPES.MDX.JSX_ATTRIBUTE, {
       name,
-      value: createTree(AST_NODES.MDX.JSX_ATTRIBUTE_EXPRESSION, {
+      value: createTree(AST_NODE_TYPES.MDX.JSX_ATTRIBUTE_EXPRESSION, {
         data: {
           estree: {
-            type: AST_NODES.ESTREE.PROGRAM,
+            type: AST_NODE_TYPES.ESTREE.PROGRAM,
             body: [
               {
-                type: AST_NODES.ESTREE.EXPRESSION_STATEMENT,
+                type: AST_NODE_TYPES.ESTREE.EXPRESSION_STATEMENT,
                 expression: valueToEstree(value),
               },
             ],
@@ -70,8 +72,8 @@ function createAttributeNode(name, value) {
 
   // For primitives, use simple string conversion.
   // If undefined, pass nothing.
-  return createTree(AST_NODES.MDX.JSX_ATTRIBUTE, {
+  return createTree(AST_NODE_TYPES.MDX.JSX_ATTRIBUTE, {
     name,
-    value: value === undefined ? value : String(value),
+    value: value == null ? value : String(value),
   });
 }

src/generators/jsx/utils/buildBarProps.mjs

@@ -1,5 +1,6 @@
 import readingTime from 'reading-time';
 import { visit } from 'unist-util-visit';
+import { DOC_API_BLOB_EDIT_BASE_URL } from '../../../constants.mjs';
 
 /**
  * Builds sidebar navigation for API documentation pages
@@ -47,6 +48,6 @@ export const buildMetaBarProps = (head, entries) => {
     addedIn: head.introduced_in || head.added_in || '',
     readingTime: readingTime(textContent).text,
     viewAs: [['JSON', `${head.api}.json`]],
-    editThisPage: `${head.edit_link}`,
+    editThisPage: `${DOC_API_BLOB_EDIT_BASE_URL}${head.api}.md`,
   };
 };

src/generators/jsx/utils/buildContent.mjs

@@ -7,41 +7,24 @@ import { SKIP, visit } from 'unist-util-visit';
 import createQueries from '../../../utils/queries/index.mjs';
 import { createJSXElement } from './ast.mjs';
 import {
-  ICON_SYMBOL_MAP,
+  API_ICONS,
+  AST_NODE_TYPES,
   STABILITY_LEVELS,
-  CHANGE_TYPES,
+  LIFECYCLE_LABELS,
+  INTERNATIONALIZABLE,
 } from '../constants.mjs';
 import { DOC_NODE_BLOB_BASE_URL } from '../../../constants.mjs';
 import { enforceArray, sortChanges } from '../../../utils/generators.mjs';
 import { buildMetaBarProps } from './buildBarProps.mjs';
 
-/**
- * Transforms a stability node into an AlertBox JSX element
- *
- * @param {import('mdast').Blockquote} node - The stability node to transform
- * @param {number} index - The index of the node in its parent's children array
- * @param {import('unist').Parent} parent - The parent node containing the stability node
- * @returns {[typeof SKIP]} Visitor instruction to skip the node
- */
-const transformStabilityNode = ({ data }, index, parent) => {
-  parent.children[index] = createJSXElement('AlertBox', {
-    children: data.description,
-    level: STABILITY_LEVELS[data.index],
-    title: data.index,
-  });
-
-  return [SKIP];
-};
-
 /**
  * Creates a history of changes for an API element
- *
  * @param {ApiDocMetadataEntry} entry - The metadata entry containing change information
  * @returns {import('unist').Node|null} JSX element representing change history or null if no changes
  */
 const createChangeElement = entry => {
-  // Collect changes from version fields (added, deprecated, etc.)
-  const changeEntries = Object.entries(CHANGE_TYPES)
+  // Collect lifecycle changes (added, deprecated, etc.)
+  const changeEntries = Object.entries(LIFECYCLE_LABELS)
     // Do we have this field?
     .filter(([field]) => entry[field])
     // Get the versions as an array
@@ -59,6 +42,7 @@ const createChangeElement = entry => {
       label: change.description,
       url: change['pr-url'],
     }));
+
     changeEntries.push(...explicitChanges);
   }
 
@@ -67,14 +51,13 @@ const createChangeElement = entry => {
   }
 
   // Sort by version, newest first and create the JSX element
-  return createJSXElement('ChangeHistory', {
+  return createJSXElement(AST_NODE_TYPES.JSX.CHANGE_HISTORY, {
     changes: sortChanges(changeEntries, 'versions'),
   });
 };
 
 /**
  * Creates a source link element if a source link is available
- *
  * @param {string|undefined} sourceLink - The source link path
  * @returns {import('hastscript').Element|null} The source link element or null if no source link
  */
@@ -84,7 +67,7 @@ const createSourceLink = sourceLink => {
   }
 
   return createElement('span', [
-    'Source Code: ',
+    INTERNATIONALIZABLE.sourceCode,
     createElement(
       'a',
       { href: `${DOC_NODE_BLOB_BASE_URL}${sourceLink}` },
@@ -93,9 +76,25 @@ const createSourceLink = sourceLink => {
   ]);
 };
 
+/**
+ * Transforms a stability node into an AlertBox JSX element
+ * @param {import('mdast').Blockquote} node - The stability node to transform
+ * @param {number} index - The index of the node in its parent's children array
+ * @param {import('unist').Parent} parent - The parent node containing the stability node
+ * @returns {[typeof SKIP]} Visitor instruction to skip the node
+ */
+const transformStabilityNode = ({ data }, index, parent) => {
+  parent.children[index] = createJSXElement(AST_NODE_TYPES.JSX.ALERT_BOX, {
+    children: data.description,
+    level: STABILITY_LEVELS[data.index],
+    title: data.index,
+  });
+
+  return [SKIP];
+};
+
 /**
  * Enhances a heading node with metadata, source links, and styling
- *
  * @param {ApiDocMetadataEntry} entry - The API metadata entry
  * @param {import('mdast').Heading} node - The heading node to transform
  * @param {number} index - The index of the node in its parent's children array
@@ -106,14 +105,15 @@ const transformHeadingNode = (entry, node, index, parent) => {
   const { data, children } = node;
   const headerChildren = [
     createElement(`h${data.depth + 1}`, [
-      createElement(`a.mark#${data.slug}`, { href: `#${data.slug}` }, children),
+      createElement(`a#${data.slug}`, { href: `#${data.slug}` }, children),
     ]),
   ];
 
   // Add type icon if available
-  const iconSymbol = ICON_SYMBOL_MAP[data.type];
-  if (iconSymbol) {
-    headerChildren.unshift(createJSXElement('CircularIcon', iconSymbol));
+  if (data.type in API_ICONS) {
+    headerChildren.unshift(
+      createJSXElement(AST_NODE_TYPES.JSX.CIRCULAR_ICON, API_ICONS[data.type])
+    );
   }
 
   // Add change history if available
@@ -136,7 +136,6 @@ const transformHeadingNode = (entry, node, index, parent) => {
 
 /**
  * Processes an API documentation entry by applying transformations to its content
- *
  * @param {ApiDocMetadataEntry} entry - The API metadata entry to process
  * @returns {import('unist').Node} The processed content
  */
@@ -155,31 +154,29 @@ const processEntry = entry => {
 
 /**
  * Creates the overall content structure with processed entries
- *
  * @param {Array<ApiDocMetadataEntry>} entries - API documentation metadata entries
- * @param {Object} sideBarProps - Props for the sidebar component
- * @param {Object} metaBarProps - Props for the meta bar component
+ * @param {Record<string, any>} sideBarProps - Props for the sidebar component
+ * @param {Record<string, any>} metaBarProps - Props for the meta bar component
  * @returns {import('unist').Node} The root node of the content tree
  */
 const createContentStructure = (entries, sideBarProps, metaBarProps) => {
   return createTree('root', [
-    createJSXElement('NavBar'),
-    createJSXElement('Article', {
+    createJSXElement(AST_NODE_TYPES.JSX.NAV_BAR),
+    createJSXElement(AST_NODE_TYPES.JSX.ARTICLE, {
       children: [
-        createJSXElement('SideBar', sideBarProps),
+        createJSXElement(AST_NODE_TYPES.JSX.SIDE_BAR, sideBarProps),
         createElement('div', [
           createElement('main', entries.map(processEntry)),
-          createJSXElement('MetaBar', metaBarProps),
+          createJSXElement(AST_NODE_TYPES.JSX.META_BAR, metaBarProps),
         ]),
-        createJSXElement('Footer'),
+        createJSXElement(AST_NODE_TYPES.JSX.FOOTER),
       ],
     }),
   ]);
 };
 
 /**
  * Transforms API metadata entries into processed MDX content
- *
  * @param {Array<ApiDocMetadataEntry>} metadataEntries - API documentation metadata entries
  * @param {ApiDocMetadataEntry} head - Main API metadata entry with version information
  * @param {Object} sideBarProps - Props for the sidebar component