-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
fix(css): rewrite absolute URLs with base path in dev mode #14622
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?
Conversation
Fixes CSS url() references to public assets returning 404 in dev mode when base path is configured. Applies URL rewriting after preprocessCSS() using Vite's cssUrlRE regex pattern. Closes withastro#14585
🦋 Changeset detectedLatest commit: bbc1c54 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodSpeed Performance ReportMerging #14622 will not alter performanceComparing Summary
Footnotes |
|
looks good overall |
|
fixed the linting error |
| const css = result.css[0].code; | ||
|
|
||
| // URL should be rewritten to include base | ||
| assert.match(css, /url\(['"]?\/my-base\/fonts\/font\.woff2['"]?\)/); |
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.
Since we're checking for the value that is statically known, would this perhaps read better?
| assert.match(css, /url\(['"]?\/my-base\/fonts\/font\.woff2['"]?\)/); | |
| assert.ok(css.includes('/my-base/fonts/font.woff2')); |
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.
first test is more explicit with regex for the full syntax, later ones use .includes() for brevity. open to changing if you prefer.
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.
I somehow missed that, awesome! No need to change anything then
Changes
url()references to public assets returning 404 in dev mode whenbasepath is configuredrewriteCssUrls()function inpackages/astro/src/core/compile/style.tsthat applies URL rewriting afterpreprocessCSS()cssUrlREregex pattern from vite/packages/vite/src/node/plugins/css.ts/fonts/font.woffto/my-base/fonts/font.woffin dev mode./), external URLs (https://), data URIs, and protocol-relative URLs (//)Root cause: Vite's
preprocessCSS()API explicitly does not resolve URLs (docs). During build, Vite's CSS plugin handles URL rewriting, butin dev mode Astro calls
preprocessCSS()directly, bypassing the URL rewriting step.Closes #14585
Testing
25 unit tests added in
packages/astro/test/units/compile/css-base-path.test.jscovering:Manual testing: