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: add caching to PostCSS for regular stylesheets #6505

Merged
merged 22 commits into from
Jun 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2c8ca19
Add initial POC PostCSS cache
markdalgleish May 23, 2023
bfc0ed2
skip cache update for stale build, refactor
markdalgleish May 23, 2023
a0774a5
Merge branch 'dev' into markdalgleish/postcss-cache
markdalgleish May 24, 2023
1294acd
move file watching up to dev server
markdalgleish May 24, 2023
a3a29a4
Merge branch 'dev' into markdalgleish/postcss-cache
markdalgleish May 24, 2023
b208c7f
add tests, support build, fix watchPaths, refactor
markdalgleish May 26, 2023
c841820
Merge branch 'dev' into markdalgleish/postcss-cache
markdalgleish May 26, 2023
3591deb
Merge branch 'dev' into markdalgleish/postcss-cache
markdalgleish May 28, 2023
74b0fee
ensure all file watch paths are absolute
markdalgleish May 29, 2023
10d3066
clean up test
markdalgleish May 29, 2023
e2bc52e
clean up cache, only require one dependency set
markdalgleish May 29, 2023
017672e
clean up css imports plugin
markdalgleish May 29, 2023
5bd8a70
add changeset
markdalgleish May 29, 2023
288188f
add temporary log when updating cache value
markdalgleish May 29, 2023
e64e499
log cache get
markdalgleish May 29, 2023
7d4f4d4
normalize slashes for picomatch globs
markdalgleish May 29, 2023
9b9fc84
fix glob matcher cache
markdalgleish May 29, 2023
2a66994
clean up console logs
markdalgleish May 29, 2023
dc3ea7e
Merge branch 'dev' into markdalgleish/postcss-cache
markdalgleish Jun 1, 2023
872652a
swallow promise rejections within file watch cache
markdalgleish Jun 1, 2023
5ce6d77
remove rejected promises from cache
markdalgleish Jun 1, 2023
8f8c3aa
Merge branch 'dev' into markdalgleish/postcss-cache
markdalgleish Jun 1, 2023
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/postcss-cache-regular-stylesheets.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@remix-run/dev": major
---

Add caching to PostCSS for regular stylesheets
44 changes: 40 additions & 4 deletions integration/hmr-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test.setTimeout(120_000);
let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
config: {
serverModuleFormat: "cjs",
tailwind: true,
postcss: true,
future: {
unstable_dev: {
port: options.devPort,
Expand All @@ -39,6 +39,7 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
"cross-env": "0.0.0-local-version",
express: "0.0.0-local-version",
isbot: "0.0.0-local-version",
"postcss-import": "0.0.0-local-version",
react: "0.0.0-local-version",
"react-dom": "0.0.0-local-version",
tailwindcss: "0.0.0-local-version",
Expand Down Expand Up @@ -83,6 +84,15 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
});
`,

"postcss.config.js": js`
module.exports = {
plugins: {
"postcss-import": {}, // Testing PostCSS cache invalidation
tailwindcss: {},
}
};
`,

"tailwind.config.js": js`
/** @type {import('tailwindcss').Config} */
module.exports = {
Expand All @@ -100,6 +110,16 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
@tailwind utilities;
`,

"app/stylesWithImport.css": css`
@import "./importedStyle.css";
`,

"app/importedStyle.css": css`
.importedStyle {
font-weight: normal;
}
`,

"app/styles.module.css": css`
.test {
color: initial;
Expand All @@ -112,10 +132,12 @@ let fixture = (options: { appPort: number; devPort: number }): FixtureInit => ({
import { cssBundleHref } from "@remix-run/css-bundle";

import Counter from "./components/counter";
import styles from "./tailwind.css";
import tailwindStyles from "./tailwind.css";
import stylesWithImport from "./stylesWithImport.css";

export const links: LinksFunction = () => [
{ rel: "stylesheet", href: styles },
{ rel: "stylesheet", href: tailwindStyles },
{ rel: "stylesheet", href: stylesWithImport },
...cssBundleHref ? [{ rel: "stylesheet", href: cssBundleHref }] : [],
];

Expand Down Expand Up @@ -294,6 +316,8 @@ test("HMR", async ({ page }) => {
let originalIndex = fs.readFileSync(indexPath, "utf8");
let counterPath = path.join(projectDir, "app", "components", "counter.tsx");
let originalCounter = fs.readFileSync(counterPath, "utf8");
let importedStylePath = path.join(projectDir, "app", "importedStyle.css");
let originalImportedStyle = fs.readFileSync(importedStylePath, "utf8");
let cssModulePath = path.join(projectDir, "app", "styles.module.css");
let originalCssModule = fs.readFileSync(cssModulePath, "utf8");
let mdxPath = path.join(projectDir, "app", "routes", "mdx.mdx");
Expand All @@ -308,6 +332,15 @@ test("HMR", async ({ page }) => {
`;
fs.writeFileSync(cssModulePath, newCssModule);

// make changes to imported styles
let newImportedStyle = `
.importedStyle {
font-weight: 800;
}
`;
fs.writeFileSync(importedStylePath, newImportedStyle);

// change text, add updated styles, add new Tailwind class ("italic")
let newIndex = `
import { useLoaderData } from "@remix-run/react";
import styles from "~/styles.module.css";
Expand All @@ -318,7 +351,7 @@ test("HMR", async ({ page }) => {
const t = useLoaderData();
return (
<main>
<h1 className={styles.test}>Changed</h1>
<h1 className={styles.test + ' italic importedStyle'}>Changed</h1>
</main>
)
}
Expand All @@ -332,13 +365,16 @@ test("HMR", async ({ page }) => {
await h1.waitFor({ timeout: HMR_TIMEOUT_MS });
expect(h1).toHaveCSS("color", "rgb(255, 255, 255)");
expect(h1).toHaveCSS("background-color", "rgb(0, 0, 0)");
expect(h1).toHaveCSS("font-style", "italic");
expect(h1).toHaveCSS("font-weight", "800");

// verify that `<input />` value was persisted (i.e. hmr, not full page refresh)
expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
await page.waitForSelector(`#root-counter:has-text("inc 1")`);

// undo change
fs.writeFileSync(indexPath, originalIndex);
fs.writeFileSync(importedStylePath, originalImportedStyle);
fs.writeFileSync(cssModulePath, originalCssModule);
await page.getByText("Index Title").waitFor({ timeout: HMR_TIMEOUT_MS });
expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
"lodash": "^4.17.21",
"npm-run-all": "^4.1.5",
"patch-package": "^6.5.0",
"postcss-import": "^15.1.0",
"prettier": "^2.7.1",
"prompt-confirm": "^2.0.4",
"react": "^18.2.0",
Expand Down
5 changes: 4 additions & 1 deletion packages/remix-dev/cli/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import { TaskError } from "../codemod/utils/task";
import { transpile as convertFileToJS } from "./useJavascript";
import { warnOnce } from "../warnOnce";
import type { Options } from "../compiler/options";
import { createFileWatchCache } from "../compiler/fileWatchCache";

export async function create({
appTemplate,
Expand Down Expand Up @@ -180,8 +181,10 @@ export async function build(
options.devOrigin = origin;
}

let fileWatchCache = createFileWatchCache();

fse.emptyDirSync(config.assetsBuildDirectory);
await compiler.build({ config, options }).catch((thrown) => {
await compiler.build({ config, options, fileWatchCache }).catch((thrown) => {
compiler.logThrown(thrown);
process.exit(1);
});
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/compiler/context.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import type { RemixConfig } from "../config";
import type { FileWatchCache } from "./fileWatchCache";
import type { Options } from "./options";

export type Context = {
config: RemixConfig;
options: Options;
fileWatchCache: FileWatchCache;
};
196 changes: 196 additions & 0 deletions packages/remix-dev/compiler/fileWatchCache.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
import picomatch from "picomatch";
import path from "path";

type CacheValue = {
cacheValue: string;
} & (
| { fileDependencies?: Set<string>; globDependencies: Set<string> }
| { fileDependencies: Set<string>; globDependencies?: Set<string> }
);

export interface FileWatchCache {
get(key: string): Promise<CacheValue> | undefined;
set(key: string, promise: Promise<CacheValue>): Promise<CacheValue>;
/**
* #description Get a cache value, or lazily set the value if it doesn't exist
* and then return the new cache value. This lets you interact with the cache
* in a single expression.
*/
getOrSet(
key: string,
lazySetter: () => Promise<CacheValue>
): Promise<CacheValue>;
invalidateFile(path: string): void;
}

const globMatchers = new Map<string, ReturnType<typeof picomatch>>();
function getGlobMatcher(glob: string) {
let matcher = globMatchers.get(glob);

if (!matcher) {
matcher = picomatch(normalizeSlashes(glob));
globMatchers.set(glob, matcher);
}

return matcher;
}

export function createFileWatchCache(): FileWatchCache {
let promiseForCacheKey = new Map<string, Promise<CacheValue>>();

let fileDepsForCacheKey = new Map<string, Set<string>>();
let cacheKeysForFileDep = new Map<string, Set<string>>();

// Glob dependencies are primarily here to support Tailwind.
// Tailwind directives like `@tailwind utilities` output a bunch of
// CSS that changes based on the usage of class names in any file matching
// the globs specified in the `content` array in the Tailwind config, so
// those globs become a dependency of any CSS file using these directives.
let globDepsForCacheKey = new Map<string, Set<string>>();
let cacheKeysForGlobDep = new Map<string, Set<string>>();

function invalidateCacheKey(invalidatedCacheKey: string): void {
// If it's not a cache key (or doesn't have a cache entry), bail out
if (!promiseForCacheKey.has(invalidatedCacheKey)) {
return;
}

promiseForCacheKey.delete(invalidatedCacheKey);

// Since we keep track of the mapping between cache key and file
// dependencies, we clear all references to the invalidated cache key.
// These will be repopulated when "set" or "getOrSet" are called.
let fileDeps = fileDepsForCacheKey.get(invalidatedCacheKey);
if (fileDeps) {
for (let fileDep of fileDeps) {
cacheKeysForFileDep.get(fileDep)?.delete(invalidatedCacheKey);
}
fileDepsForCacheKey.delete(invalidatedCacheKey);
}

// Since we keep track of the mapping between cache key and glob
// dependencies, we clear all references to the invalidated cache key.
// These will be repopulated when "set" or "getOrSet" are called.
let globDeps = globDepsForCacheKey.get(invalidatedCacheKey);
if (globDeps) {
for (let glob of globDeps) {
cacheKeysForGlobDep.get(glob)?.delete(invalidatedCacheKey);
}
globDepsForCacheKey.delete(invalidatedCacheKey);
}
}

function invalidateFile(invalidatedFile: string): void {
// Invalidate all cache entries that depend on the file.
let cacheKeys = cacheKeysForFileDep.get(invalidatedFile);
if (cacheKeys) {
for (let cacheKey of cacheKeys) {
invalidateCacheKey(cacheKey);
}
}

// Invalidate all cache entries that depend on a glob that matches the file.
// Any glob could match the file, so we have to check all globs.
for (let [glob, cacheKeys] of cacheKeysForGlobDep) {
let match = getGlobMatcher(glob);
if (match && match(normalizeSlashes(invalidatedFile))) {
for (let cacheKey of cacheKeys) {
invalidateCacheKey(cacheKey);
}
}
}
}

function get(key: string): Promise<CacheValue> | undefined {
return promiseForCacheKey.get(key);
}

function set(key: string, promise: Promise<CacheValue>): Promise<CacheValue> {
promiseForCacheKey.set(key, promise);

promise
.catch(() => {
// Swallow errors to prevent the build from crashing and remove the
// rejected promise from the cache so consumers can retry
if (promiseForCacheKey.get(key) === promise) {
promiseForCacheKey.delete(key);
}

return null;
})
.then((promiseValue) => {
// If the promise was rejected, don't attempt to track dependencies
if (promiseValue === null) {
return;
}

if (promiseForCacheKey.get(key) !== promise) {
// This cache key was invalidated before the promise resolved
// so we don't want to track the dependencies.
return;
}

let { fileDependencies, globDependencies } = promiseValue;

// Track all file dependencies for this entry point so we can invalidate
// all cache entries that depend on a file that was invalidated.
if (fileDependencies) {
let fileDeps = fileDepsForCacheKey.get(key);
if (!fileDeps) {
fileDeps = new Set();
fileDepsForCacheKey.set(key, fileDeps);
}
for (let fileDep of fileDependencies) {
fileDeps.add(fileDep);

let cacheKeys = cacheKeysForFileDep.get(fileDep);
if (!cacheKeys) {
cacheKeys = new Set();
cacheKeysForFileDep.set(fileDep, cacheKeys);
}
cacheKeys.add(key);
}
}

// Track all glob dependencies for this entry point so we can invalidate
// all cache entries that depend on a glob that matches the invalided file.
if (globDependencies) {
let globDeps = globDepsForCacheKey.get(key);
if (!globDeps) {
globDeps = new Set();
globDepsForCacheKey.set(key, globDeps);
}
for (let glob of globDependencies) {
globDeps.add(glob);

let cacheKeys = cacheKeysForGlobDep.get(glob);
if (!cacheKeys) {
cacheKeys = new Set();
cacheKeysForGlobDep.set(glob, cacheKeys);
}
cacheKeys.add(key);
}
}
});

return promise;
}

function getOrSet(
key: string,
lazySetter: () => Promise<CacheValue>
): Promise<CacheValue> {
return promiseForCacheKey.get(key) || set(key, lazySetter());
}

return {
get,
set,
getOrSet,
invalidateFile,
};
}

function normalizeSlashes(file: string) {
return file.split(path.win32.sep).join("/");
}
Loading