fix(cli): #1723 gate import.meta.url transform on js content-type so wasm skips acorn#1724
Open
jstockdi wants to merge 1 commit into
Conversation
…content-type so wasm skips acorn
This file contains hidden or 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
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.
Related Issue
Resolves #1723
Documentation
N/A — no user-facing documentation changes. (The supported wasm pattern,
new URL("./x.wasm", import.meta.url), is unchanged; this PR only fixes the crash for the unsupported direct-import syntax.)Summary of Changes
greenwood-import-meta-urlRolluptransformso it only parses a module with acorn when the final served response content-type is JavaScript. Binary resources (.wasm, and by extension.png/.woff2/ etc.) are no longer fed to the JS parser, sogreenwood buildno longer crashes with a cryptic acornSyntaxErrorattributed to internal plugin code.canTransformflag was settruewhenever any resource plugin served the module; because the node-modules resource plugin serves any existing file astext/javascript, wasm binaries slipped through. The fix keepscanTransformand additionally requiresresponse.headers.get("content-type")to containtext/javascriptbefore parsing.rollup.config.js:366):response?.headers?.get("content-type") || "".indexOf("text/javascript") >= 0was truthy for any content-type; parenthesized to(response?.headers?.get("content-type") || "").indexOf("text/javascript") >= 0so it actually tests that the content-type containstext/javascript.packages/cli/test/cases/:build.default.wasm-import-meta-url— the supportednew URL("./add.wasm", import.meta.url)pattern: asserts the build succeeds, emits exactly one content-hashed.wasmasset that is byte-identical to the source, and that the bundled script references the hashed filename.build.default.wasm-import-invalid— an unsupported directimport ... from "./add.wasm": asserts the build still fails (this syntax is not supported), but no longer crashes with the acornSyntaxErrorfrom thegreenwood-import-meta-urlplugin. Post-fix the direct import surfaces a clear, Rollup-nativePARSE_ERRORinstead of a stack pointing at Greenwood internals.Resulting behavior for a direct
.wasmimportThe direct
import x from "./x.wasm"syntax remains unsupported and the build fails, but the failure is now a Rollup-nativePARSE_ERROR(Rollup parsing the served wasm bytes as a module) rather than the misleading acorn crash insidegreenwood-import-meta-url. Users should use the supportednew URL("./x.wasm", import.meta.url)+fetch/WebAssembly.instantiateStreamingpattern, which content-hashes the asset correctly.Breaking changes
None. The supported
new URL(...)wasm path is unchanged (verified byte-for-byte), and legitimate JavaScript modules still transform normally (their served content-type istext/javascript).