nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.85k stars 29.72k forks source link

detect-module: confusing error when parsing a CommonJS module with top-level `await` #55776

Open GeoffreyBooth opened 1 week ago

GeoffreyBooth commented 1 week ago

Porting from https://github.com/nodejs/TSC/issues/1445#issuecomment-2388678002:

index.js:

const {
    getPort,
    checkPort,
    getRandomPort,
    waitForPort,
} = require("get-port-please")

const port = await getPort()

Getting this:

Restarting 'index.js'
(node:15356) [MODULE_TYPELESS_PACKAGE_JSON] Warning: Module type of file:///C:/Users/Babak/Documents/Code/c12/index.js is not specified and it doesn't parse as CommonJS.
Reparsing as ES module because module syntax was detected. This incurs a performance overhead.
To eliminate this warning, add "type": "module" to C:\Users\Babak\Documents\Code\c12\package.json.
(Use `node --trace-warnings ...` to show where the warning was created)
file:///C:/Users/Babak/Documents/Code/c12/index.js:7
} = require("get-port-please")
    ^

ReferenceError: require is not defined in ES module scope, you can use import instead
    at file:///C:/Users/Babak/Documents/Code/c12/index.js:7:5
    at ModuleJob.run (node:internal/modules/esm/module_job:262:25)
    at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:483:26)

Node.js v22.9.0
Failed running 'index.js'

With this package.json:

{
    "dependencies": {
        "express": "4.21.0",
        "get-port-please": "3.1.2"
    },
    "devDependencies": {
        "@types/express": "5.0.0"
    }
}

LOL. (await 😈).

Originally posted by @babakfp in https://github.com/nodejs/TSC/issues/1445#issuecomment-2388678002


So basically, this module fails to parse as either CommonJS or as ESM, and we show the ESM parsing error message. Perhaps we should show both, or show a special message for the common use case of using top-level await in a CommonJS module.

mertcanaltin commented 1 week ago

Hi, I've looked around a bit to work on this and I wonder where I can find the CommonJS parsing error.

https://github.com/nodejs/node/blob/main/lib/internal/modules/esm/module_job.js#L286

I think it's here: 216. line

if (format === 'commonjs') {
          const importStatement = splitStack[1];
          // TODO(@ctavan): The original error stack only provides the single
          // line which causes the error. For multi-line import statements we
          // cannot generate an equivalent object destructuring assignment by
          // just parsing the error stack.
          const oneLineNamedImports = RegExpPrototypeExec(/{.*}/, importStatement);
          const destructuringAssignment = oneLineNamedImports &&
            RegExpPrototypeSymbolReplace(/\s+as\s+/g, oneLineNamedImports, ': ');
          e.message = `Named export '${name}' not found. The requested module` +
            ` '${childSpecifier}' is a CommonJS module, which may not support` +
            ' all module.exports as named exports.\nCommonJS modules can ' +
            'always be imported via the default export, for example using:' +
            `\n\nimport pkg from '${childSpecifier}';\n${
              destructuringAssignment ?
                `const ${destructuringAssignment} = pkg;\n` : ''}`;
          const newStack = StringPrototypeSplit(e.stack, '\n');
          newStack[3] = `SyntaxError: ${e.message}`;
          e.stack = ArrayPrototypeJoin(newStack, '\n');
        }
GeoffreyBooth commented 1 week ago

I wonder where I can find the CommonJS parsing error.

https://github.com/nodejs/node/blob/main/src/node_contextify.cc#L1613-L1614

mertcanaltin commented 2 days ago

Hello, I tried to make a small improvement for this place, I would be very happy if you review it at the appropriate time.