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

fix(dev): cache PostCSS for side-effect imports #6554

Merged
merged 8 commits into from
Jun 14, 2023
5 changes: 5 additions & 0 deletions .changeset/postcss-cache-side-effect-imports.md
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
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
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
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
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
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