fix: avoid inlining raw/url CSS imports #9925

gtm-nayan · 2023-05-15T02:14:26Z

I pulled it out of one the vite tests but the ?direct option doesn't seem well documented. I'll add tests and changesets after a resolution on this.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.

Tests

Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

changeset-bot · 2023-05-15T02:14:29Z

🦋 Changeset detected

Latest commit: fdc95c9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

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

elliott-with-the-longest-name-on-github · 2023-05-15T05:25:17Z

Does this fix #9923 as well? Looks like it might be the same cause...

gtm-nayan · 2023-05-15T05:59:01Z

Whoops, I was checking #9923 with a sligtly different CSS and realized this doesn't fully solve the original problem either an import with ?url also inlines the CSS, which causes FOSC instead 😅

dominikg · 2023-05-15T07:41:55Z

packages/kit/src/exports/vite/dev/index.js

@@ -176,20 +176,18 @@ export async function dev(vite, vite_config, svelte_config) {
 							const styles = {};

 							for (const dep of deps) {
-								const url = new URL(dep.url, 'http://localhost/');
+								if (/[?&](?:raw|url|inline)\b/.test(dep.url)) continue;


this has a slight chance for false positives, move the check to after url parse and test query?

Vite also does a loose check like this, so I don't think we need to refactor this. Though now it's checking by the search params instead which I'm also indifferent.

dominikg · 2023-05-15T07:43:15Z

Is this behavior correct or could there be cases where inlining these would be expected/wanted?

gtm-nayan · 2023-05-15T07:55:33Z

Kit also inlines CSS dependencies from dynamically imported modules and dynamically imported CSS, #7229 that's one case.

Edit: Ahh sorry, read your message the wrong way around, I don't think there's any.

packages/kit/test/apps/basics/test/cross-platform/test.js

benmccann · 2023-05-15T15:08:08Z

packages/kit/src/exports/vite/dev/index.js

@@ -176,20 +176,17 @@ export async function dev(vite, vite_config, svelte_config) {
 							const styles = {};

 							for (const dep of deps) {
-								const url = new URL(dep.url, 'http://localhost/');
+								const url = new URL(dep.url, 'dummy:/');


should dummy:/ be dummy://? why doesn't localhost work?

should dummy:/ be dummy://?

why doesn't localhost work?

changed it while tinkering around but figured I'd keep it since I think it conveys the purpose better.

packages/kit/src/exports/vite/dev/index.js

* feat: Add a speedier script tag for prerendered redirects (sveltejs#9911) * feat: Add a script redirect to prerendered pages * changeset * Update .changeset/hungry-rocks-hunt.md Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> * fix test * feat: Update string escaping * Update .changeset/hungry-rocks-hunt.md Co-authored-by: Conduitry <git@chor.date> --------- Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Conduitry <git@chor.date> * fix: avoid inlining raw/url CSS imports (sveltejs#9925) * fix: use transformRequest for CSS modules * just avoid raw or url * test and changeset * use parsed query * remove only * Update packages/kit/test/apps/basics/test/cross-platform/test.js * drive by test speed up * feat: prerender & analyse in worker rather than subprocess to support Deno (sveltejs#9919) * feat(fork): use workers * Create hot-actors-hope.md * Update packages/kit/src/utils/fork.js Co-authored-by: Rich Harris <hello@rich-harris.dev> --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <hello@rich-harris.dev> * avoid using isMainThread, since it interacts poorly with vitest (sveltejs#9941) Co-authored-by: Rich Harris <git@rich-harris.dev> * chore: bump vite and devalue (sveltejs#9933) * bump vite and devalue * update templates * merge master * fix test --------- Co-authored-by: Rich Harris <git@rich-harris.dev> * chore: uvu -> vitest for create-svelte tests (sveltejs#9910) * chore: uvu -> vitest for create-svelte tests * format * concurrency (sveltejs#9921) * realised we werent typechecking this file, found some errors. fixed * Wait for beforeAll hook to complete * simplify * exclude create-svelte/template files from prettier, so that we can emit correctly formatted templates * remove unused file * Revert "exclude create-svelte/template files from prettier, so that we can emit correctly formatted templates" This reverts commit aa188d4. --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <git@rich-harris.dev> * feat: unshadow `form` and `data` in `enhance` (sveltejs#9902) * feat: Un-shadow `data` and `form` in `enhance`, warn about future deprecation in dev * changeset * snek * Update .changeset/odd-crews-own.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * Update packages/kit/test/apps/dev-only/package.json Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * am not smart * still not smart * oops * oof * add deprecation notice --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <git@rich-harris.dev> * fix: Set loader: { '.wasm': 'copy' } in esbuild config in `adapter-cloudflare-workers` (sveltejs#9940) * fix: Set loader: { '.wasm': 'copy' } in esbuild config in `adapter-cloudflare-workers` Copies WASM files in Cloudflare instead of trying to load them. Related to sveltejs#9909 * Create brave-peaches-buy.md * Update packages/adapter-cloudflare-workers/index.js * format --------- Co-authored-by: Rich Harris <git@rich-harris.dev> Co-authored-by: Rich Harris <hello@rich-harris.dev> * fix: Flaky test (sveltejs#9947) * fix: Flaky test * reuse locator * add semi * drive by test speed up * another classic * oh my god brain, y u so bad --------- Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> Co-authored-by: gtmnayan <gtmnayan@gmail.com> * remove envVarsInUse (sveltejs#9942) Co-authored-by: Rich Harris <git@rich-harris.dev> * fix: type `vitePlugin` in config (sveltejs#9946) * fix: type `vitePlugin` in config * changeset * fix: Set `loader: { '.wasm': 'copy' }` in esbuild config in `adapter-vercel` (sveltejs#9944) * fix: Enable wasm copy in adapter-vercel * Create olive-rings-eat.md --------- Co-authored-by: Rich Harris <richard.a.harris@gmail.com> * feat: crawl URLs in `<meta>` tags (sveltejs#9900) * Crawl social-image urls during prerender * Formatting & Linting * Format changeset & added exhaustive list of crawlable urls * Changed severity to minor as described in sveltejs#5228 * Added support for `property` attribute & limited valid names to just social tags * More tests * Better changeset message - I'm indecisive * Update .changeset/thirty-garlics-tan.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * simplify * simplify * Removed redundant data-sanitation * DRY out --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <git@rich-harris.dev> * Version Packages (sveltejs#9893) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * feat: support AWS via SST in adapter-auto (sveltejs#9874) * [feat] support AWS via SST in adapter-auto * Sync * Delete 95-adapter-aws-sst.md * Update .changeset/rotten-ducks-tan.md --------- Co-authored-by: Rich Harris <hello@rich-harris.dev> * Version Packages (sveltejs#9953) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * only cache response if response has cache-control header (sveltejs#9885) * only cache response if response has cache-control header * add changeset * Version Packages (sveltejs#9955) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * fix: ensure styles are loaded in dev mode for routes containing special characters (sveltejs#9894) * Fix loading styles for routes containing special characters in dev mode. SvelteKit doesn't decode special characters in pathnames when loading CSS modules in dev mode, resulting in an error: Internal server error: Failed to load url /src/routes/(special)/hinnap%C3%A4ring/+page.svelte?svelte=&type=style&lang.css=&inline= (resolved id: /src/routes/(special)/hinnap%C3%A4ring/+page.svelte?svelte&type=style&lang.css). Does the file exist? Actual path: /src/routes/(special)/hinnapäring/ Fix by using decodeURI on the url.pathname when loading CSS modules. * Create stale-houses-yell.md * decodeURL inside if * add test --------- Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <git@rich-harris.dev> * feat: Warn users when submitting forms with files but no `enctype="multipart/form-data"` (sveltejs#9888) * fix: Package name keeps me from filtering with pnpm * feat: Warn users when submitting a form containing a file without the correct enctype * changeset * Update packages/kit/src/runtime/app/forms.js Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> * style * moar style tweaks * better test skip * Update .changeset/tasty-llamas-relate.md Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> * DRY out * only warn once per submit * Update .changeset/tasty-llamas-relate.md Co-authored-by: Rich Harris <richard.a.harris@gmail.com> --------- Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <git@rich-harris.dev> Co-authored-by: Rich Harris <richard.a.harris@gmail.com> * Version Packages (sveltejs#9957) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> * security: Stop automatically adding URLs from server-side `load` `fetch` calls to dependencies (sveltejs#9945) * feat: Add `dangerZone` config * breaking: Don't implicitly track deps in server-side fetch * changeset * bein dumb * fix: Server load invalidation * fix: Write config for server * fix: test * unsurprisingly i am dumb * docs: Clarify difference between server `fetch` and universal `fetch` * tests * rename to trackServerFetches --------- Co-authored-by: Rich Harris <git@rich-harris.dev> * Version Packages (sveltejs#9963) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> --------- Co-authored-by: S. Elliott Johnson <sejohnson@torchcloudconsulting.com> Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com> Co-authored-by: Conduitry <git@chor.date> Co-authored-by: gtmnayan <50981692+gtm-nayan@users.noreply.github.com> Co-authored-by: Fernando López Guevara <fernando.lguevara@gmail.com> Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> Co-authored-by: Rich Harris <hello@rich-harris.dev> Co-authored-by: Rich Harris <richard.a.harris@gmail.com> Co-authored-by: Rich Harris <git@rich-harris.dev> Co-authored-by: Conner <conner@semicognitive.com> Co-authored-by: gtmnayan <gtmnayan@gmail.com> Co-authored-by: Loris Sigrist <43482866+LorisSigrist@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Frank <frank@sst.dev> Co-authored-by: Frank Dumont <fj.dumont@gmail.com> Co-authored-by: Reio Remma <reio@mrstuudio.ee>

fix: use transformRequest for CSS modules

1c49515

gtm-nayan requested review from bluwy and dominikg May 15, 2023 02:14

gtm-nayan marked this pull request as draft May 15, 2023 05:59

just avoid raw or url

7fef50e

vercel bot deployed to Preview – kit May 15, 2023 06:57 View deployment

vercel bot deployed to Preview – kit-svelte-dev May 15, 2023 07:00 View deployment

test and changeset

37d42cd

vercel bot deployed to Preview – kit May 15, 2023 07:10 View deployment

gtm-nayan marked this pull request as ready for review May 15, 2023 07:10

gtm-nayan changed the title ~~fix: use transformRequest for inlined CSS~~ fix: avoid inlining raw/url CSS imports May 15, 2023

vercel bot deployed to Preview – kit-svelte-dev May 15, 2023 07:12 View deployment

dominikg reviewed May 15, 2023

View reviewed changes

gtm-nayan added 2 commits May 15, 2023 14:08

use parsed query

ec5bc78

remove only

e74e086

vercel bot deployed to Preview – kit May 15, 2023 08:24 View deployment

vercel bot deployed to Preview – kit-svelte-dev May 15, 2023 08:30 View deployment

gtm-nayan commented May 15, 2023

View reviewed changes

packages/kit/test/apps/basics/test/cross-platform/test.js Outdated Show resolved Hide resolved

Update packages/kit/test/apps/basics/test/cross-platform/test.js

852517d

vercel bot deployed to Preview – kit May 15, 2023 10:10 View deployment

vercel bot deployed to Preview – kit-svelte-dev May 15, 2023 10:13 View deployment

gtm-nayan added 2 commits May 15, 2023 16:11

drive by test speed up

46897ae

Merge branch 'css-url' of https://github.com/sveltejs/kit into css-url

fdc95c9

vercel bot deployed to Preview – kit May 15, 2023 10:33 View deployment

vercel bot deployed to Preview – kit-svelte-dev May 15, 2023 10:33 View deployment

benmccann reviewed May 15, 2023

View reviewed changes

bluwy reviewed May 15, 2023

View reviewed changes

packages/kit/src/exports/vite/dev/index.js Show resolved Hide resolved

bluwy approved these changes May 16, 2023

View reviewed changes

Rich-Harris merged commit 50acb22 into master May 16, 2023

Rich-Harris deleted the css-url branch May 16, 2023 18:39

github-actions bot mentioned this pull request May 16, 2023

Version Packages #9893

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: avoid inlining raw/url CSS imports #9925

fix: avoid inlining raw/url CSS imports #9925

gtm-nayan commented May 15, 2023 •

edited

Loading

changeset-bot bot commented May 15, 2023 •

edited

Loading

elliott-with-the-longest-name-on-github commented May 15, 2023

gtm-nayan commented May 15, 2023

dominikg May 15, 2023

bluwy May 15, 2023

dominikg commented May 15, 2023

gtm-nayan commented May 15, 2023 •

edited

Loading

benmccann May 15, 2023

gtm-nayan May 15, 2023

fix: avoid inlining raw/url CSS imports #9925

fix: avoid inlining raw/url CSS imports #9925

Conversation

gtm-nayan commented May 15, 2023 • edited Loading

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

Tests

Changesets

changeset-bot bot commented May 15, 2023 • edited Loading

🦋 Changeset detected

elliott-with-the-longest-name-on-github commented May 15, 2023

gtm-nayan commented May 15, 2023

dominikg May 15, 2023

Choose a reason for hiding this comment

bluwy May 15, 2023

Choose a reason for hiding this comment

dominikg commented May 15, 2023

gtm-nayan commented May 15, 2023 • edited Loading

benmccann May 15, 2023

Choose a reason for hiding this comment

gtm-nayan May 15, 2023

Choose a reason for hiding this comment

gtm-nayan commented May 15, 2023 •

edited

Loading

changeset-bot bot commented May 15, 2023 •

edited

Loading

gtm-nayan commented May 15, 2023 •

edited

Loading