code review (1)

Author: avivkeller

src/generators/legacy-json-all/index.mjs

@@ -4,6 +4,9 @@ import { writeFile } from 'node:fs/promises';
 import { join } from 'node:path';
 
 /**
+ * This generator consolidates data from the `legacy-json` generator into a single
+ * JSON file (`all.json`).
+ *
  * @typedef {Array<import('../legacy-json/types.d.ts').Section>} Input
  *
  * @type {import('../types.d.ts').GeneratorMetadata<Input, import('./types.d.ts').Output>}
@@ -20,6 +23,9 @@ export default {
 
   async generate(input, { output }) {
     /**
+     * The consolidated output object that will contain
+     * combined data from all sections in the input.
+     *
      * @type {import('./types.d.ts').Output}
      */
     const generatedValue = {

src/generators/legacy-json/utils/buildSection.mjs

@@ -134,6 +134,7 @@ function parseListItem(child, entry) {
 
   // Recursively parse nested options if a list is found within the list item
   const optionsNode = child.children.find(child => child.type === 'list');
+
   if (optionsNode) {
     current.options = optionsNode.children.map(child =>
       parseListItem(child, entry)
@@ -175,6 +176,7 @@ function parseStability(section, nodes) {
  */
 function parseList(section, nodes, entry) {
   const list = nodes[0]?.type === 'list' ? nodes.shift() : null;
+
   const values = list
     ? list.children.map(child => parseListItem(child, entry))
     : [];
@@ -188,7 +190,8 @@ function parseList(section, nodes, entry) {
     case 'property':
       if (values.length) {
         const { type, ...rest } = values[0];
-        if (type) section.propertySigType = type;
+
+        section.type = type;
         Object.assign(section, rest);
         section.textRaw = `\`${section.name}\` ${section.textRaw}`;
       }
@@ -197,26 +200,35 @@ function parseList(section, nodes, entry) {
       section.params = values;
       break;
     default:
-      if (list) nodes.unshift(list); // If the list wasn't processed, add it back for further processing
+      // If the list wasn't processed, add it back for further processing
+      if (list) {
+        nodes.unshift(list);
+      }
   }
 }
 
+let lazyHTML;
+
 /**
  * Adds a description to the section.
  * @param {import('../types.d.ts').Section} section - The section to add description to.
  * @param {Array} nodes - The AST nodes.
  */
 function addDescription(section, nodes) {
-  if (!nodes.length) return;
+  if (!nodes.length) {
+    return;
+  }
 
   if (section.desc) {
     section.shortDesc = section.desc;
   }
 
-  const html = getRemarkRehype();
-  const rendered = html.stringify(
-    html.runSync({ type: 'root', children: nodes })
+  lazyHTML ??= getRemarkRehype();
+
+  const rendered = lazyHTML.stringify(
+    lazyHTML.runSync({ type: 'root', children: nodes })
   );
+
   section.desc = rendered || undefined;
 }
 
@@ -241,32 +253,38 @@ function addAdditionalMetadata(section, parentSection, headingNode) {
  */
 function addToParent(section, parentSection) {
   const pluralType = sectionTypePlurals[section.type];
+
   parentSection[pluralType] = parentSection[pluralType] || [];
   parentSection[pluralType].push(section);
 }
 
+const notTransferredKeys = ['textRaw', 'name', 'type', 'desc', 'miscs'];
+
 /**
- * Promotes children to top-level if the section type is 'misc'.
+ * Promotes children properties to the parent level if the section type is 'misc'.
+ *
  * @param {import('../types.d.ts').Section} section - The section to promote.
  * @param {import('../types.d.ts').Section} parentSection - The parent section.
  */
 const makeChildrenTopLevelIfMisc = (section, parentSection) => {
-  if (section.type !== 'misc' || parentSection.type === 'misc') {
-    return;
+  // Only promote if the current section is of type 'misc' and the parent is not 'misc'
+  if (section.type === 'misc' && parentSection.type !== 'misc') {
+    Object.entries(section).forEach(([key, value]) => {
+      // Skip keys that should not be transferred
+      if (notTransferredKeys.includes(key)) return;
+
+      // Merge the section's properties into the parent section
+      parentSection[key] = parentSection[key]
+        ? // If the parent already has this key, concatenate the values
+          [].concat(parentSection[key], value)
+        : // Otherwise, directly assign the section's value to the parent
+          value;
+    });
   }
+};
 
-  Object.keys(section).forEach(key => {
-    if (['textRaw', 'name', 'type', 'desc', 'miscs'].includes(key)) {
-      return;
-    }
-    if (parentSection[key]) {
-      parentSection[key] = Array.isArray(parentSection[key])
-        ? parentSection[key].concat(section[key])
-        : section[key];
-    } else {
-      parentSection[key] = section[key];
-    }
-  });
+const handleChildren = (entry, section) => {
+  entry.hierarchyChildren?.forEach(child => handleEntry(child, section));
 };
 
 /**
@@ -281,19 +299,10 @@ function handleEntry(entry, parentSection) {
   parseStability(section, nodes);
   parseList(section, nodes, entry);
   addDescription(section, nodes);
-  entry.hierarchyChildren?.forEach(child => handleEntry(child, section));
+  handleChildren(entry, section);
   addAdditionalMetadata(section, parentSection, headingNode);
   addToParent(section, parentSection);
   makeChildrenTopLevelIfMisc(section, parentSection);
-
-  if (section.type === 'property') {
-    if (section.propertySigType) {
-      section.type = section.propertySigType;
-      delete section.propertySigType;
-    } else {
-      delete section.type;
-    }
-  }
 }
 
 /**

src/generators/legacy-json/utils/parseSignature.mjs

@@ -4,6 +4,14 @@ import { PARAM_EXPRESSION } from '../constants.mjs';
 
 const OPTIONAL_LEVEL_CHANGES = { '[': 1, ']': -1, ' ': 0 };
 
+/**
+ * @param {String} char
+ * @param {Number} depth
+ * @returns {Number}
+ */
+const updateDepth = (char, depth) =>
+  depth + (OPTIONAL_LEVEL_CHANGES[char] || 0);
+
 /**
  * @param {string} parameterName
  * @param {number} optionalDepth
@@ -14,36 +22,34 @@ function parseNameAndOptionalStatus(parameterName, optionalDepth) {
   //  We need to see if there's any leading brackets in front of the parameter
   //  name. While we're doing that, we can also get the index where the
   //  parameter's name actually starts at.
-  let startingIdx = 0;
-  for (; startingIdx < parameterName.length; startingIdx++) {
-    const levelChange = OPTIONAL_LEVEL_CHANGES[parameterName[startingIdx]];
-
-    if (!levelChange) {
-      break;
-    }
-
-    optionalDepth += levelChange;
-  }
 
+  // Find the starting index where the name begins
+  const startingIdx = [...parameterName].findIndex(
+    char => !OPTIONAL_LEVEL_CHANGES[char]
+  );
+
+  // Update optionalDepth based on leading brackets
+  optionalDepth = [...parameterName.slice(0, startingIdx)].reduce(
+    updateDepth,
+    optionalDepth
+  );
+
+  // Find the ending index where the name ends
+  const endingIdx = [...parameterName].findLastIndex(
+    char => !OPTIONAL_LEVEL_CHANGES[char]
+  );
+
+  // Update optionalDepth based on trailing brackets
+  optionalDepth = [...parameterName.slice(endingIdx + 1)].reduce(
+    updateDepth,
+    optionalDepth
+  );
+
+  // Extract the actual parameter name
+  const actualName = parameterName.slice(startingIdx, endingIdx + 1);
   const isParameterOptional = optionalDepth > 0;
 
-  // Now let's check for any trailing brackets at the end of the parameter's
-  //  name. This will tell us where the parameter's name ends.
-  let endingIdx = parameterName.length - 1;
-  for (; endingIdx >= 0; endingIdx--) {
-    const levelChange = OPTIONAL_LEVEL_CHANGES[parameterName[endingIdx]];
-    if (!levelChange) {
-      break;
-    }
-
-    optionalDepth += levelChange;
-  }
-
-  return [
-    parameterName.substring(startingIdx, endingIdx + 1),
-    optionalDepth,
-    isParameterOptional,
-  ];
+  return [actualName, optionalDepth, isParameterOptional];
 }
 
 /**
@@ -55,8 +61,8 @@ function parseDefaultValue(parameterName) {
    * @type {string | undefined}
    */
   let defaultValue;
-
   const equalSignPos = parameterName.indexOf('=');
+
   if (equalSignPos !== -1) {
     // We do have a default value, let's extract it
     defaultValue = parameterName.substring(equalSignPos).trim();
@@ -75,34 +81,29 @@ function parseDefaultValue(parameterName) {
  * @returns {import('../types.d.ts').Parameter}
  */
 function findParameter(parameterName, index, markdownParameters) {
-  let parameter = markdownParameters[index];
-  if (parameter && parameter.name === parameterName) {
+  const parameter = markdownParameters[index];
+  if (parameter?.name === parameterName) {
     return parameter;
   }
 
   // Method likely has multiple signatures, something like
   //  `new Console(stdout[, stderr][, ignoreErrors])` and `new Console(options)`
   // Try to find the parameter that this is being shared with
-  for (const markdownProperty of markdownParameters) {
-    if (markdownProperty.name === parameterName) {
-      // Found it
-      return markdownParameters;
-    } else if (markdownProperty.options) {
-      for (const option of markdownProperty.options) {
-        if (option.name === parameterName) {
-          // Found a matching one in the parameter's options
-          return Object.assign({}, option);
-        }
-      }
+  for (const property of markdownParameters) {
+    if (property.name === parameterName) {
+      return property;
     }
-  }
 
-  // At this point, we couldn't find a shared signature
-  if (parameterName.startsWith('...')) {
-    return { name: parameterName };
-  } else {
-    throw new Error(`Invalid param "${parameterName}"`);
+    const matchingOption = property.options?.find(
+      option => option.name === parameterName
+    );
+    if (matchingOption) {
+      return { ...matchingOption };
+    }
   }
+
+  // Default return if no matches are found
+  return { name: parameterName };
 }
 
 /**

src/metadata.mjs

@@ -138,10 +138,8 @@ const createMetadata = slugger => {
       internalMetadata.stability.toJSON = () =>
         internalMetadata.stability.children.map(node => node.data);
 
-      /**
-       * @type {ApiDocMetadataEntry}
-       */
-      const value = {
+      // Returns the Metadata entry for the API doc
+      return {
         api: apiDoc.stem,
         slug: sectionSlug,
         source_link,
@@ -156,15 +154,9 @@ const createMetadata = slugger => {
         stability: internalMetadata.stability,
         content: section,
         tags,
+        introduced_in,
         rawContent: apiDoc.toString(),
       };
-
-      if (introduced_in) {
-        value.introduced_in = introduced_in;
-      }
-
-      // Returns the Metadata entry for the API doc
-      return value;
     },
   };
 };

src/queries.mjs

@@ -81,6 +81,9 @@ const createQueries = () => {
       transformTypeToReferenceLink
     );
 
+    // This changes the type into a link by splitting it up
+    // into several nodes, and adding those nodes to the
+    // parent.
     const {
       children: [newNode],
     } = remark.parse(replacedTypes);

src/test/metadata.test.mjs

@@ -72,6 +72,7 @@ describe('createMetadata', () => {
       deprecated_in: undefined,
       heading,
       n_api_version: undefined,
+      introduced_in: undefined,
       rawContent: '',
       removed_in: undefined,
       slug: 'test-heading',

src/utils/parser.mjs

@@ -129,6 +129,7 @@ export const parseHeadingIntoMetadata = (heading, depth) => {
       return {
         text: heading,
         type,
+        // The highest match group should be used.
         name: matches.filter(Boolean).at(-1),
         depth,
       };