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

Disable compile time evaluation of import.meta.url #15246

Merged
merged 6 commits into from Jan 28, 2022

Conversation

pavelsavara
Copy link
Contributor

@pavelsavara pavelsavara commented Jan 25, 2022

Problem

Current webpack 5 evaluates import.meta.url to file://some/path/to/source of the build machine.

Fixes #14445

Proposed changes

add new parser setting importMeta - enable/disable import.meta parsing

module.exports = {
  module: {
    parser: {
      javascript : { importMeta: false }
    }
  }
};

Does this PR introduce a breaking change?
No

What needs to be documented once your changes are merged?

New parser.javascript option importMeta - enable/disable import.meta parsing

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 25, 2022

CLA Signed

The committers are authorized under a signed CLA.

@webpack-bot
Copy link
Contributor

webpack-bot commented Jan 25, 2022

For maintainers only:

  • This needs to be documented (issue in webpack/webpack.js.org will be filed when merged)
  • This needs to be backported to webpack 4 (issue will be created when merged)

@alexander-akait
Copy link
Member

This options should be implemented on module level, i.e. you can disable it for some modules, example of new URL(...) https://webpack.js.org/configuration/module/#moduleparserjavascripturl

- use it in ImportMetaPlugin to switch it off as necessary
@alexander-akait
Copy link
Member

alexander-akait commented Jan 25, 2022

Looks good, can you accept CLA?

@pavelsavara pavelsavara marked this pull request as ready for review January 26, 2022 08:27
@pavelsavara pavelsavara marked this pull request as draft January 26, 2022 08:36
@pavelsavara pavelsavara marked this pull request as ready for review January 26, 2022 09:49
@pavelsavara
Copy link
Contributor Author

Now I tested it that it works for my use case. I also tested that it keeps doing the original substitution by default.
I'm not sure if the other broken tests are relevant or how to fix them.
Could you please help me ?

@alexander-akait
Copy link
Member

SyntaxError: Cannot use 'import.meta' outside a module

I think it is lack supported import.meta in tests, because we use vm.runInThisContext, we can:

  • Skip test
  • Refactor our tests and support it

@alexander-akait
Copy link
Member

Run node_modules/.bin/jest test/TestCasesNormal.basictest.js

@pavelsavara
Copy link
Contributor Author

pavelsavara commented Jan 26, 2022

  • Skip test

it.skip() would not work here as it's issue with parsing the module. Any other ideas ?

  • Refactor our tests and support it

I guess that something require individual test files ?
To be able to test ES6 modules, we would need to dynamic import it instead.
That should be ok even in vm.runInThisContext I think.

And that only for tests which are for the es6 module target, because things like __filename don't work in es6 module, so we could not switch all tests.

Could you advise what enumerates the test files?
Is that jest ?
https://jestjs.io/docs/ecmascript-modules

- enable/disable import.meta parsing
- when disabled insert output.importMetaName
@vankop
Copy link
Member

vankop commented Jan 27, 2022

@pavelsavara thanks for starting PR, we discussed internally that importMeta: boolean option would be better. I have improved your PR

@vankop vankop requested a review from sokra January 27, 2022 20:19
@pavelsavara
Copy link
Contributor Author

Cool, feel free to take it over.
I wonder what fixed the SyntaxError: Cannot use 'import.meta' outside a module in the tests ?

@vankop
Copy link
Member

vankop commented Jan 28, 2022

not sure, I think syntax error was in vm context that was running without esm enabled

Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

some nitpicks

lib/config/defaults.js Outdated Show resolved Hide resolved
lib/dependencies/ImportMetaPlugin.js Outdated Show resolved Hide resolved
@webpack-bot
Copy link
Contributor

@vankop Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

@webpack-bot
Copy link
Contributor

I've created an issue to document this in webpack/webpack.js.org.

styfle added a commit to vercel/ncc that referenced this pull request Sep 30, 2022
I'm affected by webpack/webpack#14445, fixed in webpack/webpack#15246, available in webpack >= [v5.68.0](https://github.com/webpack/webpack/releases/tag/v5.68.0), so I'd love to have an ncc version using webpack >= v5.68.0!

Co-authored-by: Steven <steven@ceriously.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make import.meta.url dynamic
5 participants