Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
GeoffreyBooth opened this issue Nov 8, 2024 · 2 comments
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem.

Comments

@GeoffreyBooth
Copy link
Member

GeoffreyBooth commented Nov 8, 2024

Porting from nodejs/TSC#1445 (comment):

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 nodejs/TSC#1445 (comment)


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.

@RedYetiDev RedYetiDev added the esm Issues and PRs related to the ECMAScript Modules implementation. label Nov 8, 2024
@GeoffreyBooth GeoffreyBooth added the module Issues and PRs related to the module subsystem. label Nov 8, 2024
@GeoffreyBooth GeoffreyBooth changed the title Module syntax detection: confusing error when parsing a CommonJS module with top-level await detect-module: confusing error when parsing a CommonJS module with top-level await Nov 8, 2024
@mertcanaltin
Copy link
Member

mertcanaltin commented Nov 8, 2024

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
Copy link
Member Author

I wonder where I can find the CommonJS parsing error.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants