Skip to content
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

Exclude ?inline and ?raw from critical styles in dev #9910

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading