Skip to content

Commit

Permalink
Exclude ?inline and ?raw from critical styles in dev (#9910)
Browse files Browse the repository at this point in the history
Co-authored-by: Mark Dalgleish <mark.john.dalgleish@gmail.com>
  • Loading branch information
dj-rabel and markdalgleish authored Sep 16, 2024
1 parent 825f70e commit 6eedd68
Show file tree
Hide file tree
Showing 4 changed files with 94 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/unlucky-pigs-tap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

CSS imports with `?inline`, `?inline-css` and `?raw` are no longer incorrectly injected during SSR in development
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
- dhargitai
- dhmacs
- dima-takoy
- dj-rabel
- dmarkow
- dmillar
- DNLHC
Expand Down
60 changes: 60 additions & 0 deletions packages/remix-dev/__tests__/styles-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import { isCssUrlWithoutSideEffects } from "../vite/styles";

describe("isCssUrlWithoutSideEffects", () => {
it("returns true for query parameters that result in an exported value with no side effects", () => {
let urls = [
"my/file.css?inline",
"my/file.css?inline-css",
"my/file.css?inline&raw",
"my/file.css?raw",
"my/file.css?raw&url",
"my/file.css?url",
"my/file.css?url&something=else",
"my/file.css?something=else&url",
"my/file.css?url&raw",

// other parameters mixed in
"my/file.css?inline&something=else",
"my/file.css?something=else&inline",
"my/file.css?inline&raw&something=else",
"my/file.css?something=else&inline&raw",
"my/file.css?raw&something=else&url",
"my/file.css?something=else&raw&url",
"my/file.css?url&something=else&raw",
"my/file.css?url&raw&something=else",
"my/file.css?something=else&url&raw",
];

for (let url of urls) {
expect(isCssUrlWithoutSideEffects(url)).toBe(true);
}
});

it("returns false for other query parameters or no parameters", () => {
let urls = [
"my/file.css",
"my/file.css?foo",
"my/file.css?foo=bar",
"my/file.css?foo&bar",
"my/file.css?inlinex",
"my/file.css?rawx",
"my/file.css?urlx",

// values other than blank since Vite doesn't match these
"my/file.css?inline=foo",
"my/file.css?inline-css=foo",
"my/file.css?raw=foo",
"my/file.css?url=foo",

// explicitly blank values since Vite doesn't match these
"my/file.css?inline=",
"my/file.css?inline-css=",
"my/file.css?raw=",
"my/file.css?url=",
];

for (let url of urls) {
expect(isCssUrlWithoutSideEffects(url)).toBe(false);
}
});
});
29 changes: 28 additions & 1 deletion packages/remix-dev/vite/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,33 @@ const cssModulesRegExp = new RegExp(`\\.module${cssFileRegExp.source}`);
const isCssFile = (file: string) => cssFileRegExp.test(file);
export const isCssModulesFile = (file: string) => cssModulesRegExp.test(file);

// https://vitejs.dev/guide/features#disabling-css-injection-into-the-page
// https://github.com/vitejs/vite/blob/561b940f6f963fbb78058a6e23b4adad53a2edb9/packages/vite/src/node/plugins/css.ts#L194
// https://vitejs.dev/guide/features#static-assets
// https://github.com/vitejs/vite/blob/561b940f6f963fbb78058a6e23b4adad53a2edb9/packages/vite/src/node/utils.ts#L309-L310
const cssUrlParamsWithoutSideEffects = ["url", "inline", "raw", "inline-css"];
export const isCssUrlWithoutSideEffects = (url: string) => {
let queryString = url.split("?")[1];

if (!queryString) {
return false;
}

let params = new URLSearchParams(queryString);
for (let paramWithoutSideEffects of cssUrlParamsWithoutSideEffects) {
if (
// Parameter is blank and not explicitly set, i.e. "?url", not "?url="
params.get(paramWithoutSideEffects) === "" &&
!url.includes(`?${paramWithoutSideEffects}=`) &&
!url.includes(`&${paramWithoutSideEffects}=`)
) {
return true;
}
}

return false;
};

const getStylesForFiles = async ({
viteDevServer,
rootDirectory,
Expand Down Expand Up @@ -71,7 +98,7 @@ const getStylesForFiles = async ({
if (
dep.file &&
isCssFile(dep.file) &&
!dep.url.endsWith("?url") // Ignore styles that resolved as URLs, otherwise we'll end up injecting URLs into the style tag contents
!isCssUrlWithoutSideEffects(dep.url) // Ignore styles that resolved as URLs, inline or raw. These shouldn't get injected.
) {
try {
let css = isCssModulesFile(dep.file)
Expand Down

0 comments on commit 6eedd68

Please sign in to comment.