registerHooks: TypeError when json file is required in hook and in the imported file

[timokoessler]

Version

v23.9.0

Platform

Darwin Mac 24.3.0 Darwin Kernel Version 24.3.0: Thu Jan  2 20:24:23 PST 2025; root:xnu-11215.81.4~3/RELEASE_ARM64_T8122 arm64

Subsystem

No response

What steps will reproduce the bug?

Create the following files:

instrument.js

const mod = require("module");

mod.registerHooks({
  load(url, context, nextLoad) {
    const result = nextLoad(url, context);
    if (context.format === "json") {
      // We don't want to modify the JSON file
      return result;
    }

    // Normally we would extract the path to the package from the url and read the root package.json
    console.log(
      `Patching package with version ${require("./package.json").version}`
    );

    return result;
  },
});

app

console.log("Hello from app.js");
console.log(`My version is "${require("./package.json").version}"`);

package.json

{
  "version": "1.0.0",
  "type": "commonjs"
}

Run node --import ./instrument.js ./app.js

How often does it reproduce? Is there a required condition?

Always, as long as the same json file is imported using require.

What is the expected behavior? Why is that the expected behavior?

Script does not throw an exception and outputs:

Patching package with version 1.0.0
Hello from app.js
My version is "1.0.0"

What do you see instead?

node --import ./instrument.js ./app.js
Patching package with version 1.0.0
Hello from app.js
node:internal/modules/esm/translators:151
    return cjsCache.get(job.url).exports;
                                ^

TypeError: Cannot read properties of undefined (reading 'exports')
    at require (node:internal/modules/esm/translators:151:33)
    at Object.<anonymous> (/Users/timokoessler/Git/nodejs-module-hooks-bug/package-json/app.js:2:31)
    at loadCJSModule (node:internal/modules/esm/translators:165:3)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/translators:204:7)
    at ModuleJob.run (node:internal/modules/esm/module_job:273:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:600:26)
    at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:98:5)

Additional information

The package.json file is required during the hook to check if we support the installed package version, but the issue is not limited to package.json files and happens with all json files.

Workaround: Use JSON.parse(readFileSync(...))

[joyeecheung]

If you run it with node --require ./instrument.js ./app.js it would run as normal. --import uses the ESM loader which re-invents the require() function in imported CJS files that triggers the hooks in a way that's not working quite naturally with the CJS loader.

In essence I think the fix would still be similar to #57327 (comment) - revert the approach in #47999 and do not re-invent the require() function in the ESM loader. That has been causing several other issues as well.

[Radiergummi]

Without deeper knowledge of node internals—wouldn't the obvious solution be to return a Proxy instance for the module with an exports prop that defers to the module? I think it's reasonable that this code:

const { version } = require('../package.json');

would be loaded as:

import packageJson from '../package.json' with { type: 'json' };
const version = packageJson.version;

[joyeecheung]

IIUC you are suggesting to return a Proxy as the ESM namespace object, that is not supported by the JavaScript language by design, the creation of the namespace object is handled by the JS engine and Node.js only gets to mutate its properties during module evaluation, but there is no way for the host to replace it with a Proxy (again, by design of ESM, that part is in the JS spec, and out of the hands of Node.js)

[joyeecheung]

I think this may have already been fixed by #59929 - it no longer reproduces in v24.11.1 or v25.1.0. Closing.