[@rollup/plugin-terser] esm module uses __filename

[jakearchibald]

• Rollup Plugin Name: @rollup/plugin-terser
• Rollup Plugin Version: 0.2.0
• Rollup Version: 3.6.0
• Link to reproduction (⚠️ read below): https://github.com/rollup/plugins/blob/master/packages/terser/src/module.ts#L9

Error:

[!] ReferenceError: __filename is not defined
ReferenceError: __filename is not defined
    at terser (file:///…/node_modules/@rollup/plugin-terser/dist/es/index.js:121:19)

Should use import.meta.url instead.

[shellscape]

@jakearchibald for future reference, please keep all bits of the templates in the issues.

[jakearchibald]

It felt redundant, but:

Expected Behavior

Run without throwing an error.

Actual Behavior

Threw the following error:

[!] ReferenceError: __filename is not defined
ReferenceError: __filename is not defined
    at terser (file:///…/node_modules/@rollup/plugin-terser/dist/es/index.js:121:19)

[shellscape]

All good. @tada5hi is the lead for that plugin, so hopefully they see this soon

[josdejong]

For reference: I think I'm hitting this issue, when upgrading @rollup/plugin-terser from 0.1.0 to 0.2.0 in an ES project with "type": "module". I get the following error:

[!] RollupError: Node tried to load your configuration as an ES module even though it is likely CommonJS. To resolve this, change the extension of your configuration to ".cjs" or pass the "--bundleConfigAsCjs" flag.

Original error: __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 'C:\...\package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
https://rollupjs.org/guide/en/#--bundleconfigascjs
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 'C:\...\package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at terser (file:///C:/.../node_modules/@rollup/plugin-terser/dist/es/index.js:121:19)
    at file:///C:/.../rollup.config.bundle.js:50:19
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at importModuleDynamicallyWrapper (node:internal/vm/module:438:15)
    at getConfigFileExport (C:\...\node_modules\rollup\dist\shared\loadConfigFile.js:432:17)
    at Object.loadConfigFile (C:\...\node_modules\rollup\dist\shared\loadConfigFile.js:391:59)
    at getConfigs (C:\...\node_modules\rollup\dist\bin\rollup:1680:39)
    at runRollup (C:\...\node_modules\rollup\dist\bin\rollup:1657:43)

[tada5hi]

I will take care of the problem ✌️

[mxdvl]

Hey @tada5hi, you might want to check this isomorphic __dirname blog post.

EDIT: saw you raised #1367 for a fix

[shellscape]

@mxdvl that approach also throws an error with Node 16+, tested that last night

[yoku2010]

To fix this issue for now.

I put the below lines of code in my rollup.config.js file before using terser() function

import { fileURLToPath } from 'url';
const __filename = fileURLToPath(import.meta.url);
global['__filename'] = __filename;

[brettz9]

To fix this issue for now.

I put the below lines of code in my rollup.config.js file before using terser() function

import { fileURLToPath } from 'url';
const __filename = fileURLToPath(import.meta.url);
global['__filename'] = __filename;

This avoids the __filename error for me, but now I get the error:

Error: Minify worker stopped with exit code 1

I may have some other issues present, but this doesn't clarify too much for me.

[brettz9]

This avoids the __filename error for me, but now I get the error:

Error: Minify worker stopped with exit code 1

I may have some other issues present, but this doesn't clarify too much for me.

The error appears to be: DefaultsError: 'maxWorkers' is not a supported option it seems that, while the terser rollup plugin allows the option, terser itself doesn't?

a temporary workaround would be to remove the maxWorkers option from your config file, or if you really want to use multiple workers, you could use trick like this: replace the maxWorkers: <number>, property in your config file with __proto__: {maxWorkers: 10}, (this works because the code which checks for invalid options will ignore inherited properties)

Thanks. It appears in my case that the config is instead a comments function I had. Removing that causes it to work.

[jonkoops]

Got a fix available for this under #1374?, can I get a review from a code owner?

[michaelfaith]

Got a fix available for this under #1367, can I get a review from a code owner?

Do you mean #1374?

[jonkoops]

Correct, copied the wrong PR nr. Fixed.

[shellscape]

I think collaboration on the original PR fix would have been a better route to go. @tada5hi will have to weigh in.

[tada5hi]

@shellscape I can confirm that using ESNEXT or ES2020 prevents the import.meta syntax from being recognized as an error by tsc and the code is transpiled to cjs require syntax by rollup on build time.

I also updated my pr, but im totally fine merging the pr (#1374) of @jonkoops.
Besides, i also prevented the plugin from passing unknown options to the underlying terser lib (thx @12Me21 for the hint), but i can also fix this in another pr.

[jonkoops]

Works for me either way, whatever is your preference 👍

[badfeather]

For me, this allows it to run, but throws a (!) Broken sourcemap warning:

import { fileURLToPath } from 'url';
const __filename = fileURLToPath(import.meta.url);
global['__filename'] = __filename;

[12Me21]

For me, this allows it to run, but throws a (!) Broken sourcemap warning:

I'm guessing it's because it sets __filename to the same name in every file
if you define a getter instead, it can return the correct filename everywhere:

import { fileURLToPath } from 'url'
Object.defineProperty(global, '__filename', {
	get() { return fileURLToPath(import.meta.url) },
})

[BR0kEN-]

import { fileURLToPath } from 'url'
Object.defineProperty(global, '__filename', {
	get() { return fileURLToPath(import.meta.url) },
})

This gives the same value every time because import.meta in the current file always points to the current file.

[BR0kEN-]

What you really want is:

import { fileURLToPath } from 'url';

// @todo: Delete the stuff below once the issue is resolved.
// @see https://github.com/rollup/plugins/issues/1366
const errorStackMatchFileUrlRegexp = /^.+?\((.+?):\d+:\d+\)$/;

Object.defineProperty(global, '__filename', {
  get: () => fileURLToPath(
    // Get the callee from the stack.
    // - [0]: always the `'Error'` string;
    // - [1]: always the current file;
    // - [2]: always the file where the `__filename` global has been accessed.
    // 'Error',
    // '    at get (file:///path/to/rollup.config.mjs:13:7)',
    // '    at terser (file:///path/to/node_modules/@rollup/plugin-terser/dist/es/index.js:121:19)',
    new Error()
      .stack
      .split('\n')
      .at(2)
      .replace(errorStackMatchFileUrlRegexp, '$1'),
  ),
});

[tada5hi]

@shellscape do you think we can merge one of the pull requests? If the problem with the source-map should still be a problem, i would like to address that in another pr, if you are fine with that.

[Bloodsucker]

What you really want is:

import { fileURLToPath } from 'url';

// @todo: Delete the stuff below once the issue is resolved.
// @see https://github.com/rollup/plugins/issues/1366
const errorStackMatchFileUrlRegexp = /^.+?\((.+?):\d+:\d+\)$/;

Object.defineProperty(global, '__filename', {
  get: () => fileURLToPath(
    // Get the callee from the stack.
    // - [0]: always the `'Error'` string;
    // - [1]: always the current file;
    // - [2]: always the file where the `__filename` global has been accessed.
    // 'Error',
    // '    at get (file:///path/to/rollup.config.mjs:13:7)',
    // '    at terser (file:///path/to/node_modules/@rollup/plugin-terser/dist/es/index.js:121:19)',
    new Error()
      .stack
      .split('\n')
      .at(2)
      .replace(errorStackMatchFileUrlRegexp, '$1'),
  ),
});

Am I missing something in this thread? Why would this plugin pollute the ESM global scope with a variable it does not need? The solution isn't to polyfill the __filename, which is a variable that only exist in CommonJS env, but to rewrite the code so it can use a modern ESM alternative whichever it is.

[josdejong]

Am I missing something in this thread?

The code snippets with global are workarounds you can use until the __filename issue is solved in @rollup/plugin-terser itself via #1367 or #1374.

[jonkoops]

Should we merge #1367 or #1374 and do a patch release? I think someone needs to take some action since the version that is currently out there is botched, and more users will run into this problem over time.

[tada5hi]

@jonkoops i totally agree and I would also like to release the fix as soon as possible.
Unfortunately, there are still problems with my permissions. Therefore, I can't do any new releases of the plugin at the moment, although I'm the code owner of the plugin.
So we have to wait for response and feedback from @shellscape and hope he finds time for that soon.

[shellscape]

We don't just merge to merge. We can always revert a change while a proper fix is debated and produced. opening a separate PR was a bad move because it's splintered discussion. Both PRs have unresolved discussions, and until the community and @tada5hi coalesce around one solution I'm not comfortable moving forward with either.

[tada5hi]

@shellscape you are completly right and that should definitly not be the case. I'm sorry 😔 , I forgot to mark the dicussion in #1367 as closed.
I would prefer to merge #1367, because it also solves another tiny issue, which in my opinion does not need to be addressed in a new pr.
Greetings ✌️

[jonkoops]

opening a separate PR was a bad move because it's splintered discussion

Sorry, but hard disagree, my solution was different at the time it was created. In fact, no discussion has take place on my PR so I fail to see this argument. Aside from that @tada5hi's PR now has pretty much the same implementation.

I don't feel like discussing this further, as it is quite an unproductive thing to keep talking about it. Let me make this very easy by closing my PR, so we can work on resolving the issue under @tada5hi's PR instead.

[tada5hi]

Sorry, but hard disagree, my solution was different at the time it was created. In fact, no discussion has take place on my PR so I fail to see this argument. Aside from that @tada5hi's PR now has pretty much the same implementation.

Yep. I checked in the beginning, if the __filename is defined, before falling back to import.meta. and did not directly use the import.meta variant exclusively

I don't feel like discussing this further, as it is quite an unproductive thing to keep talking about it. Let me make this very easy by closing my PR, so we can work on resolving the issue under @tada5hi's PR instead.

I got your point. I do, however, fully understand @shellscape's concerns and i prefer his approach.
It was my fault for not marking the conversation as solved and shellscape mistakenly assumed there were still open issues.

[jonkoops]

Naturally I agree with what @shellscape is saying about that PRs need to have to be properly discussed. I think that we've pretty much come to the same conclusion on what the preferred solution is at this point.

It was my fault for not marking the conversation as solved and shellscape mistakenly assumed there were still open issues.

Ah, yeah I can see where the confusion comes from there. FYI I left a couple of comments on your PR, but implementation-wise things look good to me 👍

[tada5hi]

fyi v0.2.1 is working for my esm packages.

[jonkoops]

Very nice, thanks for the effort all!