fix: keep new URL() for directory request instead of treating as asset - #14949
fix: keep new URL() for directory request instead of treating as asset#14949elecmonkey wants to merge 1 commit into
new URL() for directory request instead of treating as asset#14949Conversation
📦 Binary Size-limit
🙈 Size remains the same at 66.80MB |
Rsdoctor Bundle Diff AnalysisFound 5 projects in monorepo, 0 projects with changes. 📊 Quick Summary
Generated by Rsdoctor GitHub Action |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e771817e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if request == "." || request == ".." || request.ends_with('/') { | ||
| return Some(true); |
There was a problem hiding this comment.
When module.parser.javascript.url uses its default or relative mode and the output is a normal non-module bundle, returning Some(true) marks the entire new URL() expression as handled, so walk_new_expression never visits the second argument and ImportMetaPlugin cannot replace import.meta.url. The generated classic-script/CommonJS .js therefore retains new URL('.', import.meta.url), which is a syntax error outside an ES module; the preservation needs to be restricted to an ESM-compatible mode or the base argument must still be processed for other modes.
Useful? React with 👍 / 👎.
9d44987 to
4e77181
Compare
Deploying rspack with
|
| Latest commit: |
4e77181
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://faddedbb.rspack-v2.pages.dev |
| Branch Preview URL: | https://elecmonkey-new-url-directory.rspack-v2.pages.dev |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 877372f705
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // `import.meta.url` handling applies. | ||
| if request == "." || request == ".." || request.ends_with('/') { | ||
| if args.len() == 2 { | ||
| parser.walk_expression(&args[1].expr); |
There was a problem hiding this comment.
When the first argument evaluates to a directory through a parser-time replacement—for example, DefinePlugin({ DIR: JSON.stringify(".") }) with new URL(DIR, import.meta.url)—get_url_request sees ".", but this branch returns true after walking only the base argument. Returning true suppresses the normal walk of all arguments, so DefinePlugin's expression hook never emits the replacement and the bundle retains an undefined DIR, causing a runtime ReferenceError. Since this branch preserves the expression, it must also walk args[0].
Useful? React with 👍 / 👎.
defc42a to
a21c963
Compare
a21c963 to
af814c2
Compare
1 participant
Summary
new URL('.', import.meta.url)is a valid ESM idiom that resolves to the current module's directory (the ESM equivalent of__dirname).Rspack's
URLPlugintreated anynew URL(string, import.meta.url)as an asset reference, so a directory request (.,.., or a trailing-slash path) was wrongly resolved to a module and emitted as a static asset, and the expression was rewritten to point at that emitted file — breaking the directory semantics.Fix
Skip creating a
URLDependencywhen the request is a directory (./../ ends with/) and keepnew URL(...)as-is, soimport.meta.urlis preserved and the expression evaluates to the directory at runtime.This aligns with webpack, whose
URLParserPluginhas the same guard (isDirectoryRequest→keepNewURL): https://github.com/webpack/webpack/blob/9d017167aa3d8818e23281b2683f0f081ce0a7d0/lib/url/URLParserPlugin.js#L68-L87Reference for the standard behavior:
new URL()is defined by the WHATWG URL Standard: https://url.spec.whatwg.org/#dom-url-url././/../../resolution derives from RFC 3986 §5.4.1, which resolves them to a directory (a trailing-slash URI), not a file: https://www.rfc-editor.org/rfc/rfc3986#section-5.4.1