Skip to content

Commit

Permalink
fix(remix-dev): handle absolute bundled CSS URLs (#5788)
Browse files Browse the repository at this point in the history
  • Loading branch information
markdalgleish authored Mar 30, 2023
1 parent f059de8 commit 9b5cb43
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/absolute-css-urls.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

Fix absolute paths in CSS `url()` rules when using CSS Modules, Vanilla Extract and CSS side-effect imports
46 changes: 46 additions & 0 deletions integration/css-modules-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ test.describe("CSS Modules", () => {
...rootRelativeImportedValueFixture(),
...imageUrlsFixture(),
...rootRelativeImageUrlsFixture(),
...absoluteImageUrlsFixture(),
...clientEntrySideEffectsFixture(),
...deduplicatedCssFixture(),
...uniqueClassNamesFixture(),
Expand Down Expand Up @@ -541,6 +542,51 @@ test.describe("CSS Modules", () => {
expect(imgStatus).toBe(200);
});

let absoluteImageUrlsFixture = () => ({
"app/routes/absolute-image-urls-test.jsx": js`
import { Test } from "~/test-components/absolute-image-urls";
export default function() {
return <Test />;
}
`,
"app/test-components/absolute-image-urls/index.jsx": js`
import styles from "./styles.module.css";
export function Test() {
return (
<div data-testid="absolute-image-urls" className={styles.root}>
Image URLs test
</div>
);
}
`,
"app/test-components/absolute-image-urls/styles.module.css": css`
.root {
background-color: peachpuff;
background-image: url(/absolute-image-urls/image.svg);
padding: ${TEST_PADDING_VALUE};
}
`,
"public/absolute-image-urls/image.svg": `
<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
<circle cx="50" cy="50" r="50" fill="coral" />
</svg>
`,
});
test("absolute image URLs", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
let imgStatus: number | null = null;
app.page.on("response", (res) => {
if (res.url().endsWith(".svg")) imgStatus = res.status();
});
await app.goto("/absolute-image-urls-test");
let locator = await page.locator("[data-testid='absolute-image-urls']");
let backgroundImage = await locator.evaluate(
(element) => window.getComputedStyle(element).backgroundImage
);
expect(backgroundImage).toContain(".svg");
expect(imgStatus).toBe(200);
});

let clientEntrySideEffectsFixture = () => ({
"app/entry.client.jsx": js`
import { RemixBrowser } from "@remix-run/react";
Expand Down
41 changes: 41 additions & 0 deletions integration/css-side-effect-imports-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ test.describe("CSS side-effect imports", () => {
...rootRelativeFixture(),
...imageUrlsFixture(),
...rootRelativeImageUrlsFixture(),
...absoluteImageUrlsFixture(),
...commonJsPackageFixture(),
...esmPackageFixture(),
},
Expand Down Expand Up @@ -206,6 +207,46 @@ test.describe("CSS side-effect imports", () => {
expect(imgStatus).toBe(200);
});

let absoluteImageUrlsFixture = () => ({
"app/absoluteImageUrls/styles.css": css`
.absoluteImageUrls {
background-color: peachpuff;
background-image: url(/absoluteImageUrls/image.svg);
padding: ${TEST_PADDING_VALUE};
}
`,
"public/absoluteImageUrls/image.svg": `
<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
<circle cx="50" cy="50" r="50" fill="coral" />
</svg>
`,
"app/routes/absolute-image-urls-test.jsx": js`
import "../absoluteImageUrls/styles.css";
export default function() {
return (
<div data-testid="absolute-image-urls" className="absoluteImageUrls">
Absolute image URLs test
</div>
)
}
`,
});
test("absolute image URLs", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
let imgStatus: number | null = null;
app.page.on("response", (res) => {
if (res.url().endsWith(".svg")) imgStatus = res.status();
});
await app.goto("/absolute-image-urls-test");
let locator = await page.locator("[data-testid='absolute-image-urls']");
let backgroundImage = await locator.evaluate(
(element) => window.getComputedStyle(element).backgroundImage
);
expect(backgroundImage).toContain(".svg");
expect(imgStatus).toBe(200);
});

let commonJsPackageFixture = () => ({
"node_modules/@test-package/commonjs/styles.css": css`
.commonJsPackage {
Expand Down
45 changes: 45 additions & 0 deletions integration/vanilla-extract-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ test.describe("Vanilla Extract", () => {
...stableIdentifiersFixture(),
...imageUrlsViaCssUrlFixture(),
...imageUrlsViaRootRelativeCssUrlFixture(),
...imageUrlsViaAbsoluteCssUrlFixture(),
...imageUrlsViaJsImportFixture(),
...imageUrlsViaRootRelativeJsImportFixture(),
...imageUrlsViaClassCompositionFixture(),
Expand Down Expand Up @@ -416,6 +417,50 @@ test.describe("Vanilla Extract", () => {
expect(imgStatus).toBe(200);
});

let imageUrlsViaAbsoluteCssUrlFixture = () => ({
"app/fixtures/imageUrlsViaAbsoluteCssUrl/styles.css.ts": js`
import { style } from "@vanilla-extract/css";
export const root = style({
backgroundColor: 'peachpuff',
backgroundImage: 'url("/imageUrlsViaAbsoluteCssUrl/image.svg")',
padding: ${JSON.stringify(TEST_PADDING_VALUE)}
});
`,
"public/imageUrlsViaAbsoluteCssUrl/image.svg": `
<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
<circle cx="50" cy="50" r="50" fill="coral" />
</svg>
`,
"app/routes/image-urls-via-absolute-css-url-test.jsx": js`
import * as styles from "../fixtures/imageUrlsViaAbsoluteCssUrl/styles.css";
export default function() {
return (
<div data-testid="image-urls-via-absolute-css-url" className={styles.root}>
Image URLs via absolute CSS URL test
</div>
)
}
`,
});
test("image URLs via absolute CSS URL", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
let imgStatus: number | null = null;
app.page.on("response", (res) => {
if (res.url().endsWith(".svg")) imgStatus = res.status();
});
await app.goto("/image-urls-via-absolute-css-url-test");
let locator = await page.locator(
"[data-testid='image-urls-via-absolute-css-url']"
);
let backgroundImage = await locator.evaluate(
(element) => window.getComputedStyle(element).backgroundImage
);
expect(backgroundImage).toContain(".svg");
expect(imgStatus).toBe(200);
});

let imageUrlsViaJsImportFixture = () => ({
"app/fixtures/imageUrlsViaJsImport/styles.css.ts": js`
import { style } from "@vanilla-extract/css";
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/compiler/browserjs/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { CompileOptions } from "../options";
import { browserRouteModulesPlugin } from "./plugins/routes";
import { browserRouteModulesPlugin as browserRouteModulesPlugin_v2 } from "./plugins/routes_unstable";
import { cssFilePlugin } from "../plugins/cssImports";
import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin";
import { deprecatedRemixPackagePlugin } from "../plugins/deprecatedRemixPackage";
import { emptyModulesPlugin } from "../plugins/emptyModules";
import { mdxPlugin } from "../plugins/mdx";
Expand Down Expand Up @@ -117,6 +118,7 @@ const createEsbuildConfig = (
? cssSideEffectImportsPlugin({ config, options })
: null,
cssFilePlugin({ config, options }),
absoluteCssUrlsPlugin(),
externalPlugin(/^https?:\/\//, { sideEffects: false }),
mdxPlugin(config),
config.future.unstable_dev
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/compiler/css/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { getAppDependencies } from "../../dependencies";
import { loaders } from "../utils/loaders";
import type { CompileOptions } from "../options";
import { cssFilePlugin } from "../plugins/cssImports";
import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin";
import { emptyModulesPlugin } from "../plugins/emptyModules";
import { mdxPlugin } from "../plugins/mdx";
import { externalPlugin } from "../plugins/external";
Expand Down Expand Up @@ -70,6 +71,7 @@ const createEsbuildConfig = (
? cssSideEffectImportsPlugin({ config, options })
: null,
cssFilePlugin({ config, options }),
absoluteCssUrlsPlugin(),
externalPlugin(/^https?:\/\//, { sideEffects: false }),
mdxPlugin(config),
emptyModulesPlugin(config, /\.server(\.[jt]sx?)?$/),
Expand Down
23 changes: 23 additions & 0 deletions packages/remix-dev/compiler/plugins/absoluteCssUrlsPlugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import path from "path";
import type { Plugin, PluginBuild } from "esbuild";

/**
* This plugin treats absolute paths in 'url()' css rules as external to prevent
* breaking changes
*/
export const absoluteCssUrlsPlugin = (): Plugin => {
return {
name: "absolute-css-urls-plugin",
setup: async (build: PluginBuild) => {
build.onResolve({ filter: /.*/ }, async (args) => {
let { kind, path: resolvePath } = args;
if (kind === "url-token" && path.isAbsolute(resolvePath)) {
return {
path: resolvePath,
external: true,
};
}
});
},
};
};
17 changes: 2 additions & 15 deletions packages/remix-dev/compiler/plugins/cssImports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import invariant from "../../invariant";
import type { RemixConfig } from "../../config";
import type { CompileOptions } from "../options";
import { getPostcssProcessor } from "../utils/postcss";
import { absoluteCssUrlsPlugin } from "./absoluteCssUrlsPlugin";

const isExtendedLengthPath = /^\\\\\?\\/;

Expand Down Expand Up @@ -51,22 +52,8 @@ export function cssFilePlugin({
...buildOps.loader,
".css": "css",
},
// this plugin treats absolute paths in 'url()' css rules as external to prevent breaking changes
plugins: [
{
name: "resolve-absolute",
async setup(build) {
build.onResolve({ filter: /.*/ }, async (args) => {
let { kind, path: resolvePath } = args;
if (kind === "url-token" && path.isAbsolute(resolvePath)) {
return {
path: resolvePath,
external: true,
};
}
});
},
},
absoluteCssUrlsPlugin(),
...(postcssProcessor
? [postcssPlugin({ postcssProcessor, options })]
: []),
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/compiler/serverjs/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { cssModulesPlugin } from "../plugins/cssModuleImports";
import { cssSideEffectImportsPlugin } from "../plugins/cssSideEffectImports";
import { vanillaExtractPlugin } from "../plugins/vanillaExtract";
import { cssFilePlugin } from "../plugins/cssImports";
import { absoluteCssUrlsPlugin } from "../plugins/absoluteCssUrlsPlugin";
import { deprecatedRemixPackagePlugin } from "../plugins/deprecatedRemixPackage";
import { emptyModulesPlugin } from "../plugins/emptyModules";
import { mdxPlugin } from "../plugins/mdx";
Expand Down Expand Up @@ -59,6 +60,7 @@ const createEsbuildConfig = (
? cssSideEffectImportsPlugin({ config, options })
: null,
cssFilePlugin({ config, options }),
absoluteCssUrlsPlugin(),
externalPlugin(/^https?:\/\//, { sideEffects: false }),
mdxPlugin(config),
emptyModulesPlugin(config, /\.client(\.[jt]sx?)?$/),
Expand Down

0 comments on commit 9b5cb43

Please sign in to comment.