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

Error when mixing ESM and CJS #749

Open
joeldenning opened this issue Aug 13, 2021 · 10 comments
Open

Error when mixing ESM and CJS #749

joeldenning opened this issue Aug 13, 2021 · 10 comments
Assignees
Labels
bug Something isn't working

Comments

@joeldenning
Copy link

In a project with "type": "module" in the package.json that also contains .cjs files, the .cjs files cannot access the global variables __filename, __dirname, etc that should be available to commonjs files.

I've created https://github.com/joeldenning/ncc-mixed-modules to demonstrate the problem.

~/c/ncc-mixed-modules (main|✔) $ node dist/index.js
file:///Users/joeldenning/code/ncc-mixed-modules/dist/index.js:6
console.log("dep.cjs executing", __filename)
                                 ^

ReferenceError: __filename is not defined in ES module scope
This file is being treated as an ES module because it has a '.js' file extension and '/Users/joeldenning/code/ncc-mixed-modules/dist/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at Object.135 (file:///Users/joeldenning/code/ncc-mixed-modules/dist/index.js:6:34)
    at __nccwpck_require__ (file:///Users/joeldenning/code/ncc-mixed-modules/dist/index.js:32:41)
    at file:///Users/joeldenning/code/ncc-mixed-modules/dist/index.js:52:66
    at file:///Users/joeldenning/code/ncc-mixed-modules/dist/index.js:56:3
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.loadESM (node:internal/process/esm_loader:68:5)
    at async handleMainPromise (node:internal/modules/run_main:63:12)
@styfle styfle added the bug Something isn't working label Aug 13, 2021
@guybedford
Copy link
Contributor

This would be a Webpack semantic in how webpack builds CommonJS.

One of the things to note is that __filename in a bundle is not well-defined because many files with many different __filename values bundled together into a final bundle will be in a bundle file that has another __filename value. So then the question is which one of those __filename values should be used. I don't think any bundlers have a good answer to this from a definition perspective yet. Browserify used to use /local/path.js as cwd-relative turning into absolute-url-relative paths, but I'm not sure if Webpack fully adopted that convention (//cc @sokra).

So a question to understand further would be - what is the use case you have in mind here that you're looking to handle? Then we can discuss ways to approach that use case rather.

@joeldenning
Copy link
Author

In my case, I worked around it by replacing __filename with process.cwd(). The __filename was there because it was part of a generated file, and it was used as part of path.resolve(__filename, 'some-other-file.js').

@guybedford
Copy link
Contributor

Was the generated file an ES module or a CJS module?

@joeldenning
Copy link
Author

The generated file was a CJS module - it was created by sequelize-cli as part of creating a database migration. For reference, here's the original file generated by sequelize-cli:

https://github.com/JustUtahCoders/flax-crm/blob/5c35bbe3f8818ea4f936fbef09fe85c0367b98ca/backend/DB/models/index.cjs

@guybedford
Copy link
Contributor

So basically, we only rewrite __filename or __dirname when we are able to analyze what expression they are used in. If we can't do that analysis we leave them as-is and in ESM files they therefore remain undefined.

When building into CommonJS we do exactly the same thing, but __filename is then defined of course (just wrong effectively since the relative locations will break).

At the end of the day this is about use cases... I would be surprised if the use case was working with the naive CJS version anyway.

Again, regardless of outcome - this is a Webpack concern not an ncc concern. I've posted webpack/webpack#14072 to track further.

@appelgriebsch
Copy link

Looks like it also mixes up import and require statements in the bundled mjs:

screenshot-2021-10-06-162911

As such its not possible to build with the current version. The version 0.28.x builds it just fine though (only cjs output).

@algo7
Copy link

algo7 commented Jul 21, 2023

any news on this?

@styfle
Copy link
Member

styfle commented Feb 12, 2024

It looks like webpack/webpack#14072 was finally merged and released in webpack@5.90.0.

The PR to upgrade webpack (#1158) is currently failing CI so if anyone wants to fix it, I can take a look, thanks!

almeric added a commit to Appmanschap/Critical-CSS that referenced this issue Apr 12, 2024
This breaks ncc / node because of __dirname use
Maybe try again when this issue is closed:
vercel/ncc#749
@RSS1102
Copy link

RSS1102 commented May 28, 2024

It looks like webpack/webpack#14072 was finally merged and released in webpack@5.90.0.

The PR to upgrade webpack (#1158) is currently failing CI so if anyone wants to fix it, I can take a look, thanks!

transfer to #1177

@gentlementlegen
Copy link

This issue still seems relevant. __dirname is still in the output code even though the produced code should be ESM, causing the runtime to crash with ReferenceError: __dirname is not defined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants