add createRequire support #15579
add createRequire support #15579
Conversation
|
For maintainers only:
|
vankop
force-pushed
the
support-create-require
branch
from
da341a2
to
552f53a
Compare
| it("should create require", () => { | ||
| const require = _createRequire(import.meta.url); | ||
| expect(require("./a")).toBe(1); | ||
| expect(_createRequire(import.meta.url)("./c")).toBe(3); |
There was a problem hiding this comment.
TODO
add support of context. maybe contextify request?
| } | ||
| }; | ||
|
|
||
| parser.hooks.import.tap( |
There was a problem hiding this comment.
should we add parser option to enable such behaviour? (and enable it by default in target=node)
There was a problem hiding this comment.
Yes, we don't want to enable that by default in web target since it's a "node.js polyfill" which we have chosen to not provide by default.
vankop
force-pushed
the
support-create-require
branch
4 times, most recently
from
93f44bf
to
16f53cb
Compare
vankop
force-pushed
the
support-create-require
branch
from
16f53cb
to
34c4edc
Compare
| if ( | ||
| arg.type === "MemberExpression" && | ||
| arg.object.type === "MetaProperty" | ||
| ) { | ||
| if ( | ||
| arg.object.meta.type === "Identifier" && | ||
| arg.object.meta.name === "import" && | ||
| arg.object.property.type === "Identifier" && | ||
| arg.object.property.name === "meta" && | ||
| arg.property.type === "Identifier" && | ||
| arg.property.name === "url" | ||
| ) { | ||
| // same module context | ||
| return false; |
There was a problem hiding this comment.
I think we evaluate import.meta to an identifier in evaluateExpression, so this special case isn't needed in my opinion.
You can use evaluated.isIdentifier() && evaluated.identifier === "import.meta.url" for that case in the code below
| } | ||
| }; | ||
|
|
||
| parser.hooks.import.tap( |
There was a problem hiding this comment.
Yes, we don't want to enable that by default in web target since it's a "node.js polyfill" which we have chosen to not provide by default.
| expect(require.cache).toBe(_require.cache); | ||
| expect(require.cache).toBe(_createRequire(import.meta.url).cache); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Needs a test case when a string is passed to createRequire. e. g. createRequire(import.meta.url + "/../somewhere-else/file.js")
There was a problem hiding this comment.
import.meta.url + "/../somewhere-else/file.js" will not produce valid context. I have added support to new URL(...)
webpack-bot
added
PR: review-outdated
and removed
PR: reviewed-changes-requested
labels
vankop
force-pushed
the
support-create-require
branch
2 times, most recently
from
5d967db
to
64778f0
Compare
vankop
force-pushed
the
support-create-require
branch
from
64778f0
to
8df5134
Compare
# Conflicts: # declarations/WebpackOptions.d.ts # lib/config/defaults.js # schemas/WebpackOptions.check.js # schemas/WebpackOptions.json # test/Defaults.unittest.js # test/__snapshots__/Cli.basictest.js.snap # types.d.ts
|
Thanks |
What kind of change does this PR introduce?
closes #14228
Did you add tests for your changes?
yes
Does this PR introduce a breaking change?
no
What needs to be documented once your changes are merged?
nothing