-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(core): make lingui macro transform match Vite's normalized paths on Windows #2462
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "emdash": patch | ||
| --- | ||
|
|
||
| Fixes the admin UI hanging indefinitely on "Loading EmDash…" in local dev on Windows, with no visible error. The Lingui macro compiler used to build admin source in dev mode compared file paths using a Windows-style backslash path against Vite's forward-slash-normalized module ids, so the comparison always failed and macro compilation silently never ran — shipping raw, uncompiled `@lingui/*/macro` imports to the browser, which then threw during hydration. A second, previously-masked bug in the same code path passed a raw Windows drive-letter path to a dynamic `import()`, which Node's ESM loader rejects. | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -54,6 +54,19 @@ describe("createViteConfig admin aliasing", () => { | |||||||||||||||
| expect(replacement).toMatch(adminSourcePattern); | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| // Regression: Vite normalizes module ids to forward slashes (and, on | ||||||||||||||||
| // case-insensitive filesystems, to their on-disk casing) before calling | ||||||||||||||||
| // plugin hooks. The Lingui macro transform compares its own id against | ||||||||||||||||
| // this same alias path with a plain id.startsWith(adminSourcePath) check, | ||||||||||||||||
| // so a backslash-separated path here makes that check always fail — | ||||||||||||||||
| // silently skipping macro compilation for every admin source file. | ||||||||||||||||
|
Comment on lines
+57
to
+62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This block is a detailed regression narrative that largely restates the
Suggested change
|
||||||||||||||||
| it("returns the admin source path with forward slashes only", () => { | ||||||||||||||||
| const config = buildConfig(monorepoDemoRoot); | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [suggestion] This assertion only guards the regression on Windows. On Unix CI the path is already forward-slash, so reverting the normalization would still pass here and the bug could silently regress again. Consider either gating the test to Windows ( |
||||||||||||||||
| const replacement = getAdminAliasReplacement(config); | ||||||||||||||||
|
|
||||||||||||||||
| expect(replacement).not.toMatch(/\\/); | ||||||||||||||||
| }); | ||||||||||||||||
|
|
||||||||||||||||
| it("uses built admin dist for external app dev", () => { | ||||||||||||||||
| const config = buildConfig(externalProjectRoot); | ||||||||||||||||
| const replacement = getAdminAliasReplacement(config); | ||||||||||||||||
|
|
||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[needs fixing] This changeset reads like a PR description: it walks through the root cause (
Windows-style backslash path,Vite's forward-slash-normalized module ids,ERR_UNSUPPORTED_ESM_URL_SCHEME) rather than telling a user what changed.AGENTS.mdsays changesets are release notes that should describe the observable effect and leave out internal mechanics.