TypeError: Cannot add property `source`, object is not extensible

[bouzlibop]

Version

v23.7.0

Platform

Darwin MacBookPro.T-Mobile 24.3.0 Darwin Kernel Version 24.3.0: Thu Jan  2 20:24:16 PST 2025; root:xnu-11215.81.4~3/RELEASE_ARM64_T6000 arm6

Subsystem

No response

What steps will reproduce the bug?

To reproduce the issue try running the synchronous version of the transpilation example.

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

100%

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

I would expect the code from the API docs to run without any errors.

What do you see instead?

node:internal/modules/esm/load:173
    context.source = source;
                   ^

TypeError: Cannot add property source, object is not extensible

Additional information

No response

[aduh95]

/cc @joyeecheung

[joyeecheung]

It seems both the async version and the sync version of the example are incorrect:

1. Using nextLoad() on .coffee would lead to ERR_UNKNOWN_FILE_EXTENSION, which I guess is working as intended, though personally I think this is something we need to provide a mitigation for in the API (e.g. add an argument in the context to imply just reading a buffer from disk as fallback for all the unknown extensions, or TBH I am not even sure why we even need ERR_UNKNOWN_FILE_EXTENSION instead of just doing this read for all unknown extensions)
2. Both examples do not pass context to the nextLoad hook, which would lead to an error.

[joyeecheung]

Also both versions could not produce valid module types, since the documentation does not mention adding a package.json with a type field, so the module detection code would now fail with ERR_UNKNOWN_FILE_EXTENSION since it doesn't know how to detect types from .coffee (I feel that ERR_UNKNOWN_FILE_EXTENSION is really counterproductive at this point, why do we have this error? @nodejs/loaders )

[joyeecheung]

Actually I remember this was one the reasons I left

https://github.com/nodejs/node/blob/main/doc/api/module.md?plain=1#L1348-L1350

because it seems the examples have been broken for a while due to lack of testing....

[bouzlibop]

@joyeecheung I believe the async hooks version handles default context properly and sync does not. Consider following minimal example:

• package.json

  {
    "name": "test1",
    "version": "1.0.0",
    "type": "module"
  }

• a.js

  import b from './b.js';
  console.log(b);

• b.js

  export default 2;

• async-hook.js

  export async function load(url, context, nextLoad) {
   return nextLoad(url);
  }

• sync-hook.js

  import { registerHooks } from 'node:module';
  
  export function load(url, context, nextLoad) {
    return nextLoad(url);
  }
  
  registerHooks({ load });

Now running the async hook finishes without any errors:

$ node --import 'data:text/javascript,import { register } from "node:module"; import { pathToFileURL } from "node:url"; register(pathToFileURL("./async-hook.js"));' ./a.js
2

In contrast if I try to run the sync hook I get an error:

$ node --import ./sync-hook.js ./a.js
node:internal/modules/esm/load:173
    context.source = source;
                   ^

TypeError: Cannot add property source, object is not extensible

I would assume that both would have the same results. Why is that the sync version throws? 🤔

[joyeecheung]

According to discussions in #57037 the sync hooks are currently making the context parameter mandatory (as a result an internal context is created in the absence of that, which is frozen it seems). Until we fix it internally, you could make it work by always passing the context parameter to nextLoad()