Skip to content

Commit

Permalink
fix(dev): cache PostCSS for side-effect imports (#6554)
Browse files Browse the repository at this point in the history
  • Loading branch information
@markdalgleish
markdalgleish authored Jun 14, 2023
1 parent 0ab3e9a commit de44305
Show file tree
Hide file tree
Showing 7 changed files with 176 additions and 135 deletions.
5 changes: 5 additions & 0 deletions .changeset/postcss-cache-side-effect-imports.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": patch
---

Add caching to PostCSS for side-effect imports
8 changes: 8 additions & 0 deletions integration/deterministic-build-output-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,19 @@ test("builds deterministically under different paths", async () => {
// * vanillaExtractPlugin (via app/routes/foo.tsx' .css.ts file import)
let init: FixtureInit = {
config: {
postcss: true,
future: {
v2_routeConvention: true,
},
},
files: {
"postcss.config.js": js`
module.exports = {
plugins: {
"postcss-import": {},
},
};
`,
"app/routes/_index.mdx": "# hello world",
"app/routes/foo.tsx": js`
export * from "~/foo/bar.server";
Expand Down
37 changes: 35 additions & 2 deletions integration/hmr-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import getPort, { makeRange } from "get-port";
import type { FixtureInit } from "./helpers/create-fixture";
import { createFixtureProject, css, js, json } from "./helpers/create-fixture";

test.setTimeout(120_000);
test.setTimeout(150_000);

let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
config: {
Expand Down Expand Up @@ -120,6 +120,16 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
}
`,

"app/sideEffectStylesWithImport.css": css`
@import "./importedSideEffectStyle.css";
`,

"app/importedSideEffectStyle.css": css`
.importedSideEffectStyle {
font-size: initial;
}
`,

"app/styles.module.css": css`
.test {
color: initial;
Expand All @@ -134,6 +144,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
import Counter from "./components/counter";
import tailwindStyles from "./tailwind.css";
import stylesWithImport from "./stylesWithImport.css";
import "./sideEffectStylesWithImport.css";
export const links: LinksFunction = () => [
{ rel: "stylesheet", href: tailwindStyles },
Expand Down Expand Up @@ -322,6 +333,15 @@ test("HMR", async ({ page, browserName }) => {
let originalCssModule = fs.readFileSync(cssModulePath, "utf8");
let mdxPath = path.join(projectDir, "app", "routes", "mdx.mdx");
let originalMdx = fs.readFileSync(mdxPath, "utf8");
let importedSideEffectStylePath = path.join(
projectDir,
"app",
"importedSideEffectStyle.css"
);
let originalImportedSideEffectStyle = fs.readFileSync(
importedSideEffectStylePath,
"utf8"
);

// make content and style changed to index route
let newCssModule = `
Expand All @@ -340,6 +360,14 @@ test("HMR", async ({ page, browserName }) => {
`;
fs.writeFileSync(importedStylePath, newImportedStyle);

// // make changes to imported side-effect styles
let newImportedSideEffectStyle = `
.importedSideEffectStyle {
font-size: 32px;
}
`;
fs.writeFileSync(importedSideEffectStylePath, newImportedSideEffectStyle);

// change text, add updated styles, add new Tailwind class ("italic")
let newIndex = `
import { useLoaderData } from "@remix-run/react";
Expand All @@ -351,7 +379,7 @@ test("HMR", async ({ page, browserName }) => {
const t = useLoaderData();
return (
<main>
<h1 className={styles.test + ' italic importedStyle'}>Changed</h1>
<h1 className={styles.test + ' italic importedStyle importedSideEffectStyle'}>Changed</h1>
</main>
)
}
Expand All @@ -367,6 +395,7 @@ test("HMR", async ({ page, browserName }) => {
expect(h1).toHaveCSS("background-color", "rgb(0, 0, 0)");
expect(h1).toHaveCSS("font-style", "italic");
expect(h1).toHaveCSS("font-weight", "800");
expect(h1).toHaveCSS("font-size", "32px");

// verify that `<input />` value was persisted (i.e. hmr, not full page refresh)
expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
Expand All @@ -376,6 +405,10 @@ test("HMR", async ({ page, browserName }) => {
fs.writeFileSync(indexPath, originalIndex);
fs.writeFileSync(importedStylePath, originalImportedStyle);
fs.writeFileSync(cssModulePath, originalCssModule);
fs.writeFileSync(
importedSideEffectStylePath,
originalImportedSideEffectStyle
);
await page.getByText("Index Title").waitFor({ timeout: HMR_TIMEOUT_MS });
expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
await page.waitForSelector(`#root-counter:has-text("inc 1")`);
Expand Down
117 changes: 21 additions & 96 deletions packages/remix-dev/compiler/plugins/cssImports.ts
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import * as path from "path";
import * as fse from "fs-extra";
import esbuild from "esbuild";
import type { Processor } from "postcss";

import invariant from "../../invariant";
import type { Context } from "../context";
import { getPostcssProcessor } from "../utils/postcss";
import { getCachedPostcssProcessor } from "../utils/postcss";
import { absoluteCssUrlsPlugin } from "./absoluteCssUrlsPlugin";

const isExtendedLengthPath = /^\\\\\?\\/;
Expand All @@ -18,11 +17,7 @@ function normalizePathSlashes(p: string) {
* This plugin loads css files with the "css" loader (bundles and moves assets to assets directory)
* and exports the url of the css file as its default export.
*/
export function cssFilePlugin({
config,
options,
fileWatchCache,
}: Context): esbuild.Plugin {
export function cssFilePlugin(ctx: Context): esbuild.Plugin {
return {
name: "css-file",

Expand All @@ -46,7 +41,8 @@ export function cssFilePlugin({
target,
} = build.initialOptions;

let postcssProcessor = await getPostcssProcessor({ config });
// eslint-disable-next-line prefer-let/prefer-let -- Avoid needing to repeatedly check for null since const can't be reassigned
const postcssProcessor = await getCachedPostcssProcessor(ctx);

build.onLoad({ filter: /\.css$/ }, async (args) => {
let { metafile, outputFiles, warnings, errors } = await esbuild.build({
Expand All @@ -65,14 +61,14 @@ export function cssFilePlugin({
target,
treeShaking,
tsconfig,
minify: options.mode === "production",
minify: ctx.options.mode === "production",
bundle: true,
minifySyntax: true,
metafile: true,
write: false,
sourcemap: Boolean(options.sourcemap && postcssProcessor), // We only need source maps if we're processing the CSS with PostCSS
sourcemap: Boolean(ctx.options.sourcemap && postcssProcessor), // We only need source maps if we're processing the CSS with PostCSS
splitting: false,
outdir: config.assetsBuildDirectory,
outdir: ctx.config.assetsBuildDirectory,
entryNames: assetNames,
entryPoints: [args.path],
loader: {
Expand All @@ -83,11 +79,18 @@ export function cssFilePlugin({
absoluteCssUrlsPlugin(),
...(postcssProcessor
? [
postcssPlugin({
fileWatchCache,
postcssProcessor,
options,
}),
{
name: "postcss-plugin",
async setup(build) {
build.onLoad(
{ filter: /\.css$/, namespace: "file" },
async (args) => ({
contents: await postcssProcessor({ path: args.path }),
loader: "css",
})
);
},
} satisfies esbuild.Plugin,
]
: []),
],
Expand All @@ -103,13 +106,13 @@ export function cssFilePlugin({
invariant(entry, "entry point not found");

let normalizedEntry = path.resolve(
config.rootDirectory,
ctx.config.rootDirectory,
normalizePathSlashes(entry)
);
let entryFile = outputFiles.find((file) => {
return (
path.resolve(
config.rootDirectory,
ctx.config.rootDirectory,
normalizePathSlashes(file.path)
) === normalizedEntry
);
Expand Down Expand Up @@ -148,81 +151,3 @@ export function cssFilePlugin({
},
};
}

function postcssPlugin({
fileWatchCache,
postcssProcessor,
options,
}: {
fileWatchCache: Context["fileWatchCache"];
postcssProcessor: Processor;
options: Context["options"];
}): esbuild.Plugin {
return {
name: "postcss-plugin",
async setup(build) {
build.onLoad({ filter: /\.css$/, namespace: "file" }, async (args) => {
let cacheKey = `postcss-plugin?sourcemap=${options.sourcemap}&path=${args.path}`;

let { cacheValue } = await fileWatchCache.getOrSet(
cacheKey,
async () => {
let contents = await fse.readFile(args.path, "utf-8");

let { css, messages } = await postcssProcessor.process(contents, {
from: args.path,
to: args.path,
map: options.sourcemap,
});

let fileDependencies = new Set<string>();
let globDependencies = new Set<string>();

// Ensure the CSS file being passed to PostCSS is tracked as a
// dependency of this cache key since a change to this file should
// invalidate the cache, not just its sub-dependencies.
fileDependencies.add(args.path);

// PostCSS plugin result objects can contain arbitrary messages returned
// from plugins. Here we look for messages that indicate a dependency
// on another file or glob. Here we target the generic dependency messages
// returned from 'postcss-import' and 'tailwindcss' plugins, but we may
// need to add more in the future depending on what other plugins do.
// More info:
// - https://postcss.org/api/#result
// - https://postcss.org/api/#message
for (let message of messages) {
if (
message.type === "dependency" &&
typeof message.file === "string"
) {
fileDependencies.add(message.file);
continue;
}

if (
message.type === "dir-dependency" &&
typeof message.dir === "string" &&
typeof message.glob === "string"
) {
globDependencies.add(path.join(message.dir, message.glob));
continue;
}
}

return {
cacheValue: css,
fileDependencies,
globDependencies,
};
}
);

return {
contents: cacheValue,
loader: "css",
};
});
},
};
}
23 changes: 7 additions & 16 deletions packages/remix-dev/compiler/plugins/cssSideEffectImports.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { parse, type ParserOptions } from "@babel/parser";
import traverse from "@babel/traverse";
import generate from "@babel/generator";

import { getPostcssProcessor } from "../utils/postcss";
import { getCachedPostcssProcessor } from "../utils/postcss";
import { applyHMR } from "../js/plugins/hmr";
import type { Context } from "../context";

Expand Down Expand Up @@ -51,7 +51,7 @@ export const cssSideEffectImportsPlugin = (
return {
name: pluginName,
setup: async (build) => {
let postcssProcessor = await getPostcssProcessor(ctx);
let postcssProcessor = await getCachedPostcssProcessor(ctx);

build.onLoad(
{ filter: allJsFilesFilter, namespace: "file" },
Expand Down Expand Up @@ -107,21 +107,12 @@ export const cssSideEffectImportsPlugin = (
);

build.onLoad({ filter: /\.css$/, namespace }, async (args) => {
let contents = await fse.readFile(args.path, "utf8");

if (postcssProcessor) {
contents = (
await postcssProcessor.process(contents, {
from: args.path,
to: args.path,
map: ctx.options.sourcemap,
})
).css;
}

let absolutePath = path.resolve(ctx.config.rootDirectory, args.path);
return {
contents,
resolveDir: path.dirname(args.path),
contents: postcssProcessor
? await postcssProcessor({ path: absolutePath })
: await fse.readFile(absolutePath, "utf8"),
resolveDir: path.dirname(absolutePath),
loader: "css",
};
});
Expand Down
4 changes: 1 addition & 3 deletions packages/remix-dev/compiler/plugins/vanillaExtract.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,7 @@ export function vanillaExtractPlugin(

let postcssProcessor = await getPostcssProcessor({
config,
context: {
vanillaExtract: true,
},
postcssContext: { vanillaExtract: true },
});

// Resolve virtual CSS files first to avoid resolving the same
Expand Down
Loading

0 comments on commit de44305

Please sign in to comment.