Correctly read rawRequest for frontend ESM #31917
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: -2.85 kB (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
gziolo
reviewed
May 18, 2021
There was a problem hiding this comment.
The comment about the 0 B size of build/navigation/index.js in #31917 (comment) is concerning and should be further investigated.
| // being present in `entryModule.rawRequest`. | ||
| // In the context of frontend files, they would be processed | ||
| // as ESM if they use `import` or `export` within it. | ||
| const request = rootModule?.rawRequest || rawRequest; |
There was a problem hiding this comment.
I see it bring backs the handling for rawRequest removed in #30341.
There was a problem hiding this comment.
It does. It interesting because I removed it as it seemed to be a legacy way of handling modules of a different kind, shown by the fact that checksumming builds made with or without the change rendered the same result — every source file was being built into the same path structure regardless of its type.
The check becomes necessary again when building modules into arbitrary directories by using rawRequest as a starting point.
|
I can confirm that this fixes #31909 for the use-case I have put it towards. |
That file is what was being built incorrectly instead of the In the reported case, the problem was caused by |
|
Cool, it looks like everything is on track. Thank you for sharing more details. Does this branch need a rebase? The failing unit tests look completely unrelated. |
vcanales
force-pushed
the
fix/build-frontend-esm
branch
from
e1573a7
to
2eb0057
Compare
gziolo
approved these changes
May 18, 2021
vcanales
force-pushed
the
fix/build-frontend-esm
branch
2 times, most recently
from
a7a6d37
to
adb9773
Compare
vcanales
force-pushed
the
fix/build-frontend-esm
branch
from
adb9773
to
29cbf7d
Compare
|
Thank you for the change in the script that builds the zip file 💯 Let's get this in when CI finishes. |
vcanales
force-pushed
the
fix/build-frontend-esm
branch
from
29cbf7d
to
03ddda4
Compare
youknowriad
pushed a commit
that referenced
this pull request
May 26, 2021
|
Cherry-picked into the 10.7 release branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #31909
Fixes #32050
When processing ESM files, the requested path is defined in
entryModule.rootModule.rawRequest, instead of being present inentryModule.rawRequest.In the context of frontend files, they would be processed as ESM if they use
importorexportwithin it.This issue is not being flagged by the e2e pipeline, which indicates it might be dependant on the environment somehow. For example, I can reproduce this in my system by running
npm run buildbut if I usenpm run dev, everything is built correctly. e2e tests are ran on the production build, so I'm not entirely sure where the difference actually is.