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 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 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

@alexander-akait alexander-akait commented Jan 25, 2022

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 alexander-akait commented Jan 25, 2022

Looks good, can you accept CLA?

@pavelsavara pavelsavara marked this pull request as ready for review Jan 26, 2022
@pavelsavara pavelsavara marked this pull request as draft Jan 26, 2022
@pavelsavara pavelsavara marked this pull request as ready for review Jan 26, 2022
@pavelsavara
Copy link
Contributor Author

@pavelsavara pavelsavara commented Jan 26, 2022

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

@alexander-akait alexander-akait commented Jan 26, 2022

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

@alexander-akait alexander-akait commented Jan 26, 2022

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

@pavelsavara
Copy link
Contributor Author

@pavelsavara 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 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 Jan 27, 2022
@pavelsavara
Copy link
Contributor Author

@pavelsavara pavelsavara commented Jan 27, 2022

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 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

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

@webpack-bot webpack-bot commented Jan 28, 2022

@vankop Thanks for your update.

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

@sokra Please review the new changes.

sokra
sokra approved these changes Jan 28, 2022
@webpack-bot
Copy link
Contributor

@webpack-bot webpack-bot commented Apr 8, 2022

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

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.

5 participants