Repository: nodejs/node
Pull request: doc: fix transpiler loader hooks documentation

The loader hooks examples have been broken for a while:

1. The nextLoad() hook cannot be used on a .coffee file that ends
   up going to the default load step without an explict format,
   which would cause a ERR_UNKNOWN_FILE_EXTENSION. Mention
   adding a package.json with a type field to work around it
   in the example.
2. Pass the context parameter to the nextLoad() invocation and
    document that context.format is mandatory when module type
    is not explicitly inferrable from the module.
3. Correct the getPackageType() implementation which returns
   false instead of undefined in the absence of an explict format,
   which is not a valid type for format.

---

I still feel that moving the examples to a proper repository that have the examples actually tested in a workflow is the right way to go, which was why I left the TODO in L1348 instead of fixing the examples in any way some time ago. But until we do that, it's not good to have broken examples in the docs, so this just fixes it to be a working version without actually making it perfect in any way.

Also I feel that the current API is a bit awkward to use even for these examples:

1. The error ERR_UNKNOWN_FILE_EXTENSION isn't super helpful when any hooks are in place. This prevents using the next hooks for unknown extensions with format detection. It'll be more natural to either just treat unknown file extensions as .js text files by default, or make this behavior configurable in the context options.
2. There should be a helper for detecting the format based on user-provided texts + package.json types. Arguably this isn't the most optimial workflow that the customization hooks should have (we detect the type by compiling it as CJS first and then ESM, for the internal default load we can be smart enough to immediately go to subsequent steps once any of the compilation succeeds to avoid the extra compilation cost, but for the custom loader they would have to throw the module-detection compilation result away after getting the format in the load hook, but I guess at least having a helper is better than nothing)

These should probably be addressed in other PRs since they'll require changes to the API.

Refs: https://github.com/nodejs/node/issues/57030

<!--
Before submitting a pull request, please read:

- the CONTRIBUTING guide at https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md
- the commit message formatting guidelines at
  https://github.com/nodejs/node/blob/HEAD/doc/contributing/pull-requests.md#commit-message-guidelines

For code changes:
1. Include tests for any bug fixes or new features.
6. Update documentation if relevant.
7. Ensure that `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes.

If you believe this PR should be highlighted in the Node.js CHANGELOG
please add the `notable-change` label.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.
-->


Changed files:
filename: doc/api/module.md
patch: @@ -1164,7 +1164,11 @@ changes:
   Node.js default `load` hook after the last user-supplied `load` hook
   * `url` {string}
   * `context` {Object|undefined} When omitted, defaults are provided. When provided, defaults are
-    merged in with preference to the provided properties.
+    merged in with preference to the provided properties. In the default `nextLoad`, if
+    the module pointed to by `url` does not have explicit module type information,
+    `context.format` is mandatory.
+    <!-- TODO(joyeecheung): make it at least optionally non-mandatory by allowing
+         JS-style/TS-style module detection when the format is simply unknown -->
 * Returns: {Object|Promise} The asynchronous version takes either an object containing the
   following properties, or a `Promise` that will resolve to such an object. The
   synchronous version only accepts an object returned synchronously.
@@ -1362,36 +1366,32 @@ transpiler hooks should only be used for development and testing purposes.
 ```mjs
 // coffeescript-hooks.mjs
 import { readFile } from 'node:fs/promises';
-import { dirname, extname, resolve as resolvePath } from 'node:path';
-import { cwd } from 'node:process';
-import { fileURLToPath, pathToFileURL } from 'node:url';
+import { findPackageJSON } from 'node:module';
 import coffeescript from 'coffeescript';
 
 const extensionsRegex = /\.(coffee|litcoffee|coffee\.md)$/;
 
 export async function load(url, context, nextLoad) {
   if (extensionsRegex.test(url)) {
-    // CoffeeScript files can be either CommonJS or ES modules, so we want any
-    // CoffeeScript file to be treated by Node.js the same as a .js file at the
-    // same location. To determine how Node.js would interpret an arbitrary .js
-    // file, search up the file system for the nearest parent package.json file
-    // and read its "type" field.
-    const format = await getPackageType(url);
-
-    const { source: rawSource } = await nextLoad(url, { ...context, format });
+    // CoffeeScript files can be either CommonJS or ES modules. Use a custom format
+    // to tell Node.js not to detect its module type.
+    const { source: rawSource } = await nextLoad(url, { ...context, format: 'coffee' });
     // This hook converts CoffeeScript source code into JavaScript source code
     // for all imported CoffeeScript files.
     const transformedSource = coffeescript.compile(rawSource.toString(), url);
 
+    // To determine how Node.js would interpret the transpilation result,
+    // search up the file system for the nearest parent package.json file
+    // and read its "type" field.
     return {
-      format,
+      format: await getPackageType(url),
       shortCircuit: true,
       source: transformedSource,
     };
   }
 
   // Let Node.js handle all other URLs.
-  return nextLoad(url);
+  return nextLoad(url, context);
 }
 
 async function getPackageType(url) {
@@ -1402,72 +1402,51 @@ async function getPackageType(url) {
   // this simple truthy check for whether `url` contains a file extension will
   // work for most projects but does not cover some edge-cases (such as
   // extensionless files or a url ending in a trailing space)
-  const isFilePath = !!extname(url);
-  // If it is a file path, get the directory it's in
-  const dir = isFilePath ?
-    dirname(fileURLToPath(url)) :
-    url;
-  // Compose a file path to a package.json in the same directory,
-  // which may or may not exist
-  const packagePath = resolvePath(dir, 'package.json');
-  // Try to read the possibly nonexistent package.json
-  const type = await readFile(packagePath, { encoding: 'utf8' })
-    .then((filestring) => JSON.parse(filestring).type)
-    .catch((err) => {
-      if (err?.code !== 'ENOENT') console.error(err);
-    });
-  // If package.json existed and contained a `type` field with a value, voilà
-  if (type) return type;
-  // Otherwise, (if not at the root) continue checking the next directory up
-  // If at the root, stop and return false
-  return dir.length > 1 && getPackageType(resolvePath(dir, '..'));
+  const pJson = findPackageJSON(url);
+
+  return readFile(pJson, 'utf8')
+    .then(JSON.parse)
+    .then((json) => json?.type)
+    .catch(() => undefined);
 }
 ```
 
 ##### Synchronous version
 
 ```mjs
 // coffeescript-sync-hooks.mjs
-import { readFileSync } from 'node:fs/promises';
-import { registerHooks } from 'node:module';
-import { dirname, extname, resolve as resolvePath } from 'node:path';
-import { cwd } from 'node:process';
-import { fileURLToPath, pathToFileURL } from 'node:url';
+import { readFileSync } from 'node:fs';
+import { registerHooks, findPackageJSON } from 'node:module';
 import coffeescript from 'coffeescript';
 
 const extensionsRegex = /\.(coffee|litcoffee|coffee\.md)$/;
 
 function load(url, context, nextLoad) {
   if (extensionsRegex.test(url)) {
-    const format = getPackageType(url);
-
-    const { source: rawSource } = nextLoad(url, { ...context, format });
+    const { source: rawSource } = nextLoad(url, { ...context, format: 'coffee' });
     const transformedSource = coffeescript.compile(rawSource.toString(), url);
 
     return {
-      format,
+      format: getPackageType(url),
       shortCircuit: true,
       source: transformedSource,
     };
   }
 
-  return nextLoad(url);
+  return nextLoad(url, context);
 }
 
 function getPackageType(url) {
-  const isFilePath = !!extname(url);
-  const dir = isFilePath ? dirname(fileURLToPath(url)) : url;
-  const packagePath = resolvePath(dir, 'package.json');
-
-  let type;
+  const pJson = findPackageJSON(url);
+  if (!pJson) {
+    return undefined;
+  }
   try {
-    const filestring = readFileSync(packagePath, { encoding: 'utf8' });
-    type = JSON.parse(filestring).type;
-  } catch (err) {
-    if (err?.code !== 'ENOENT') console.error(err);
+    const file = readFileSync(pJson, 'utf-8');
+    return JSON.parse(file)?.type;
+  } catch {
+    return undefined;
   }
-  if (type) return type;
-  return dir.length > 1 && getPackageType(resolvePath(dir, '..'));
 }
 
 registerHooks({ load });
@@ -1489,6 +1468,21 @@ console.log "Brought to you by Node.js version #{version}"
 export scream = (str) -> str.toUpperCase()
 ```
 
+For the sake of running the example, add a `package.json` file containing the
+module type of the CoffeeScript files.
+
+```json
+{
+  "type": "module"
+}
+```
+
+This is only for running the example. In real world loaders, `getPackageType()` must be
+able to return an `format` known to Node.js even in the absence of an explicit type in a
+`package.json`, or otherwise the `nextLoad` call would throw `ERR_UNKNOWN_FILE_EXTENSION`
+(if undefined) or `ERR_UNKNOWN_MODULE_FORMAT` (if it's not a known format listed in
+the [load hook][] documentation).
+
 With the preceding hooks modules, running
 `node --import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register(pathToFileURL("./coffeescript-hooks.mjs"));' ./main.coffee`
 or `node --import ./coffeescript-sync-hooks.mjs ./main.coffee`