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

fix(terser): replace __filename with import.meta.url #1374

Closed
wants to merge 1 commit into from

Conversation

jonkoops
Copy link
Contributor

Rollup Plugin Name: terser

This PR contains:

  • bugfix
  • feature
  • refactor
  • documentation
  • other

Are tests included?

  • yes (bugfixes and features will not be merged without tests)
  • no

Breaking Changes?

  • yes (breaking changes will not be merged unless absolutely necessary)
  • no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers:

Description

Because __filename does not exist in the ES modules this plugin errors out when used in a Rollup config written in the same format. This PR replaces the __filename constant with import.meta.url instead, which is available in ES modules.

When building the CommonJS version import.meta.url is replaced with __dirname at compile time, making sure that it remains compatible.

@jonkoops
Copy link
Contributor Author

Here is an example of the transpiled CommonJS code that initializes the worker. I believe this is enough to keep backwards compatibility:

const workerPool = new WorkerPool({
    filePath: url.fileURLToPath((typeof document === 'undefined' ? new (require('u' + 'rl').URL)('file:' + __filename).href : (document.currentScript && document.currentScript.src || new URL('index.js', document.baseURI).href))),
    maxWorkers: options.maxWorkers
});

@@ -2,6 +2,7 @@
"extends": "../../tsconfig.base.json",
"include": ["src/**/*", "types/**/*"],
"compilerOptions": {
"module": "ES2020",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to set this flag, otherwise the TypeScript compiler will start complaining about using import.meta.url in something that compiles to a CommonJS target. Which of course is a non-issue as Rollup is used to compile the code, not the TypeScript compiler.

That said, should this not be the default? Then opting back into CommonJS where necessary (not even sure if that is needed).

@jonkoops
Copy link
Contributor Author

Closing this PR in favor of #1367, which has now been updated with a similar implementation.

@jonkoops jonkoops closed this Dec 21, 2022
@jonkoops jonkoops deleted the no-filename branch December 21, 2022 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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