review

Signed-off-by: flakey5 <[email protected]>

Author: flakey5

bin/cli.mjs

@@ -9,8 +9,8 @@ import { coerce } from 'semver';
 import { DOC_NODE_CHANGELOG_URL, DOC_NODE_VERSION } from '../src/constants.mjs';
 import createGenerator from '../src/generators.mjs';
 import generators from '../src/generators/index.mjs';
-import { createMarkdownLoader } from '../src/loader.mjs';
-import { createMarkdownParser } from '../src/parser.mjs';
+import createMarkdownLoader from '../src/loaders/markdown.mjs';
+import createMarkdownParser from '../src/parsers/markdown.mjs';
 import createNodeReleases from '../src/releases.mjs';
 
 const availableGenerators = Object.keys(generators);

src/generators.mjs

@@ -1,6 +1,14 @@
 'use strict';
 
-import availableGenerators from './generators/index.mjs';
+import publicGenerators from './generators/index.mjs';
+import astJs from './generators/ast-js/index.mjs';
+
+const availableGenerators = {
+  ...publicGenerators,
+  // This one is a little special since we don't want it to run unless we need
+  // it and we also don't want it to be publicly accessible through the CLI.
+  'ast-js': astJs,
+};
 
 /**
  * @typedef {{ ast: import('./generators/types.d.ts').GeneratorMetadata<ApiDocMetadataEntry, ApiDocMetadataEntry>}} AstGenerator The AST "generator" is a facade for the AST tree and it isn't really a generator
@@ -23,7 +31,7 @@ import availableGenerators from './generators/index.mjs';
  * @param {ApiDocMetadataEntry} markdownInput The parsed API doc metadata entries
  * @param {Array<import('acorn').Program>} parsedJsFiles
  */
-const createGenerator = (markdownInput, jsInput) => {
+const createGenerator = markdownInput => {
   /**
    * We store all the registered generators to be processed
    * within a Record, so we can access their results at any time whenever needed
@@ -33,7 +41,6 @@ const createGenerator = (markdownInput, jsInput) => {
    */
   const cachedGenerators = {
     ast: Promise.resolve(markdownInput),
-    'ast-js': Promise.resolve(jsInput),
   };
 
   /**

src/generators/api-links/utils/checkIndirectReferences.mjs

@@ -1,8 +1,7 @@
 import { visit } from 'estree-util-visit';
 
 /**
- *
- * @param program
+ * @param {import('acorn').Program} program
  * @param {import('../types.d.ts').ProgramExports} exports
  * @param {Record<string, number>} nameToLineNumberMap
  */

src/generators/ast-js/index.mjs

@@ -1,5 +1,5 @@
-import { createJsLoader } from '../../loader.mjs';
-import { createJsParser } from '../../parser.mjs';
+import createJsLoader from '../../loaders/javascript.mjs';
+import createJsParser from '../../parsers/javascript.mjs';
 
 /**
  * This generator parses Javascript sources passed into the generator's input
@@ -29,12 +29,8 @@ export default {
   async generate(_, options) {
     const { loadFiles } = createJsLoader();
 
-    if (!options.input) {
-      return [];
-    }
-
     // Load all of the Javascript sources into memory
-    const sourceFiles = loadFiles(options.input);
+    const sourceFiles = loadFiles(options.input ?? []);
 
     const { parseJsSources } = createJsParser();
 

src/generators/index.mjs

@@ -8,7 +8,6 @@ import legacyJson from './legacy-json/index.mjs';
 import legacyJsonAll from './legacy-json-all/index.mjs';
 import addonVerify from './addon-verify/index.mjs';
 import apiLinks from './api-links/index.mjs';
-import astJs from './ast-js/index.mjs';
 
 export default {
   'json-simple': jsonSimple,
@@ -19,5 +18,4 @@ export default {
   'legacy-json-all': legacyJsonAll,
   'addon-verify': addonVerify,
   'api-links': apiLinks,
-  'ast-js': astJs,
 };

src/generators/types.d.ts

@@ -59,6 +59,10 @@ declare global {
      *
      * The 'ast' generator is the top-level parser, and if 'ast' is passed to `dependsOn`, then the generator
      * will be marked as a top-level generator.
+     *
+     * The `ast-js` generator is the top-level parser for JavaScript files. It
+     * passes the ASTs for any JavaScript files given in the input. Like `ast`,
+     * any generator depending on it is marked as a top-level generator.
      */
     dependsOn: keyof AvailableGenerators | 'ast' | 'ast-js';
 

src/index.mjs

@@ -1,8 +1,10 @@
 export * as constants from './constants.mjs';
 export { default as generators } from './generators/index.mjs';
 export { default as createGenerator } from './generators.mjs';
-export * from './loader.mjs';
+export * from './loaders/markdown.mjs';
+export * from './loaders/javascript.mjs';
 export { default as createMetadata } from './metadata.mjs';
-export * from './parser.mjs';
+export * from './parsers/markdown.mjs';
+export * from './parsers/javascript.mjs';
 export { default as createQueries } from './queries.mjs';
 export { default as createNodeReleases } from './releases.mjs';

src/loaders/javascript.mjs

@@ -0,0 +1,33 @@
+'use strict';
+
+import { readFile } from 'node:fs/promises';
+import { extname } from 'node:path';
+
+import { globSync } from 'glob';
+import { VFile } from 'vfile';
+
+/**
+ * This creates a "loader" for loading Javascript source files into VFiles.
+ */
+const createLoader = () => {
+  /**
+   * Loads the JavaScript source files and transforms them into VFiles
+   *
+   * @param {string | Array<string>} searchPath
+   */
+  const loadFiles = searchPath => {
+    const resolvedFiles = globSync(searchPath).filter(
+      filePath => extname(filePath) === '.js'
+    );
+
+    return resolvedFiles.map(async filePath => {
+      const fileContents = await readFile(filePath, 'utf-8');
+
+      return new VFile({ path: filePath, value: fileContents });
+    });
+  };
+
+  return { loadFiles };
+};
+
+export default createLoader;

src/loader.mjs renamed to src/loaders/markdown.mjs

@@ -11,7 +11,7 @@ import { VFile } from 'vfile';
  * could be used for different things, but here we want to use it to load
  * Markdown files and transform them into VFiles
  */
-export const createMarkdownLoader = () => {
+const createLoader = () => {
   /**
    * Loads API Doc files and transforms it into VFiles
    *
@@ -36,26 +36,4 @@ export const createMarkdownLoader = () => {
   return { loadFiles };
 };
 
-/**
- * This creates a "loader" for loading Javascript source files into VFiles.
- */
-export const createJsLoader = () => {
-  /**
-   * Loads the JavaScript source files and transforms them into VFiles
-   *
-   * @param {string | Array<string>} searchPath
-   */
-  const loadFiles = searchPath => {
-    const resolvedFiles = globSync(searchPath).filter(
-      filePath => extname(filePath) === '.js'
-    );
-
-    return resolvedFiles.map(async filePath => {
-      const fileContents = await readFile(filePath, 'utf-8');
-
-      return new VFile({ path: filePath, value: fileContents });
-    });
-  };
-
-  return { loadFiles };
-};
+export default createLoader;

src/parsers/javascript.mjs

@@ -0,0 +1,55 @@
+'use strict';
+
+import * as acorn from 'acorn';
+
+/**
+ * Creates a Javascript source parser for a given source file
+ */
+const createParser = () => {
+  /**
+   * Parses a given JavaScript file into an ESTree AST representation of it
+   *
+   * @param {import('vfile').VFile | Promise<import('vfile').VFile>} sourceFile
+   * @returns {Promise<JsProgram>}
+   */
+  const parseJsSource = async sourceFile => {
+    // We allow the API doc VFile to be a Promise of a VFile also,
+    // hence we want to ensure that it first resolves before we pass it to the parser
+    const resolvedSourceFile = await Promise.resolve(sourceFile);
+
+    if (typeof resolvedSourceFile.value !== 'string') {
+      throw new TypeError(
+        `expected resolvedSourceFile.value to be string but got ${typeof resolvedSourceFile.value}`
+      );
+    }
+
+    const res = acorn.parse(resolvedSourceFile.value, {
+      allowReturnOutsideFunction: true,
+      ecmaVersion: 'latest',
+      locations: true,
+    });
+
+    return {
+      ...res,
+      path: resolvedSourceFile.path,
+    };
+  };
+
+  /**
+   * Parses multiple JavaScript files into ESTree ASTs by wrapping parseJsSource
+   *
+   * @param {Array<import('vfile').VFile | Promise<import('vfile').VFile>>} apiDocs List of API doc files to be parsed
+   * @returns {Promise<Array<JsProgram>>}
+   */
+  const parseJsSources = async apiDocs => {
+    // We do a Promise.all, to ensure that each API doc is resolved asynchronously
+    // but all need to be resolved first before we return the result to the caller
+    const resolvedApiDocEntries = await Promise.all(apiDocs.map(parseJsSource));
+
+    return resolvedApiDocEntries;
+  };
+
+  return { parseJsSource, parseJsSources };
+};
+
+export default createParser;