diff --git a/.changeset/postcss-cache-regular-stylesheets.md b/.changeset/postcss-cache-regular-stylesheets.md new file mode 100644 index 00000000000..523f2500431 --- /dev/null +++ b/.changeset/postcss-cache-regular-stylesheets.md @@ -0,0 +1,5 @@ +--- +"@remix-run/dev": major +--- + +Add caching to PostCSS for regular stylesheets diff --git a/integration/hmr-test.ts b/integration/hmr-test.ts index f4becd5ad94..ae9378782cf 100644 --- a/integration/hmr-test.ts +++ b/integration/hmr-test.ts @@ -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, @@ -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", @@ -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 = { @@ -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; @@ -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 }] : [], ]; @@ -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"); @@ -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"; @@ -318,7 +351,7 @@ test("HMR", async ({ page }) => { const t = useLoaderData(); return (
-

Changed

+

Changed

) } @@ -332,6 +365,8 @@ 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 `` value was persisted (i.e. hmr, not full page refresh) expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf"); @@ -339,6 +374,7 @@ test("HMR", async ({ page }) => { // 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"); diff --git a/package.json b/package.json index bce85e6c015..889a58e10c2 100644 --- a/package.json +++ b/package.json @@ -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", diff --git a/packages/remix-dev/cli/commands.ts b/packages/remix-dev/cli/commands.ts index 0a9074923bc..5a16e9a22cf 100644 --- a/packages/remix-dev/cli/commands.ts +++ b/packages/remix-dev/cli/commands.ts @@ -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, @@ -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); }); diff --git a/packages/remix-dev/compiler/context.ts b/packages/remix-dev/compiler/context.ts index 28f31db0050..4cdc90667bb 100644 --- a/packages/remix-dev/compiler/context.ts +++ b/packages/remix-dev/compiler/context.ts @@ -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; }; diff --git a/packages/remix-dev/compiler/fileWatchCache.ts b/packages/remix-dev/compiler/fileWatchCache.ts new file mode 100644 index 00000000000..80ed7fc7a55 --- /dev/null +++ b/packages/remix-dev/compiler/fileWatchCache.ts @@ -0,0 +1,196 @@ +import picomatch from "picomatch"; +import path from "path"; + +type CacheValue = { + cacheValue: string; +} & ( + | { fileDependencies?: Set; globDependencies: Set } + | { fileDependencies: Set; globDependencies?: Set } +); + +export interface FileWatchCache { + get(key: string): Promise | undefined; + set(key: string, promise: Promise): Promise; + /** + * #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 + ): Promise; + invalidateFile(path: string): void; +} + +const globMatchers = new Map>(); +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>(); + + let fileDepsForCacheKey = new Map>(); + let cacheKeysForFileDep = new Map>(); + + // 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>(); + let cacheKeysForGlobDep = new Map>(); + + 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 | undefined { + return promiseForCacheKey.get(key); + } + + function set(key: string, promise: Promise): Promise { + 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 + ): Promise { + return promiseForCacheKey.get(key) || set(key, lazySetter()); + } + + return { + get, + set, + getOrSet, + invalidateFile, + }; +} + +function normalizeSlashes(file: string) { + return file.split(path.win32.sep).join("/"); +} diff --git a/packages/remix-dev/compiler/plugins/cssImports.ts b/packages/remix-dev/compiler/plugins/cssImports.ts index 44640baa91c..3fb382e2b16 100644 --- a/packages/remix-dev/compiler/plugins/cssImports.ts +++ b/packages/remix-dev/compiler/plugins/cssImports.ts @@ -4,8 +4,7 @@ import esbuild from "esbuild"; import type { Processor } from "postcss"; import invariant from "../../invariant"; -import type { RemixConfig } from "../../config"; -import type { Options } from "../options"; +import type { Context } from "../context"; import { getPostcssProcessor } from "../utils/postcss"; import { absoluteCssUrlsPlugin } from "./absoluteCssUrlsPlugin"; @@ -22,10 +21,8 @@ function normalizePathSlashes(p: string) { export function cssFilePlugin({ config, options, -}: { - config: RemixConfig; - options: Options; -}): esbuild.Plugin { + fileWatchCache, +}: Context): esbuild.Plugin { return { name: "css-file", @@ -85,7 +82,13 @@ export function cssFilePlugin({ plugins: [ absoluteCssUrlsPlugin(), ...(postcssProcessor - ? [postcssPlugin({ postcssProcessor, options })] + ? [ + postcssPlugin({ + fileWatchCache, + postcssProcessor, + options, + }), + ] : []), ], }); @@ -147,28 +150,76 @@ export function cssFilePlugin({ } function postcssPlugin({ + fileWatchCache, postcssProcessor, options, }: { + fileWatchCache: Context["fileWatchCache"]; postcssProcessor: Processor; - options: Options; + options: Context["options"]; }): esbuild.Plugin { return { name: "postcss-plugin", async setup(build) { build.onLoad({ filter: /\.css$/, namespace: "file" }, async (args) => { - let contents = await fse.readFile(args.path, "utf-8"); - - contents = ( - await postcssProcessor.process(contents, { - from: args.path, - to: args.path, - map: options.sourcemap, - }) - ).css; + 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(); + let globDependencies = new Set(); + + // 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, + contents: cacheValue, loader: "css", }; }); diff --git a/packages/remix-dev/compiler/watch.ts b/packages/remix-dev/compiler/watch.ts index 0cede310af3..1920810b7a2 100644 --- a/packages/remix-dev/compiler/watch.ts +++ b/packages/remix-dev/compiler/watch.ts @@ -82,12 +82,19 @@ export async function watch( }, 100); let toWatch = [ctx.config.appDirectory]; + + // WARNING: Chokidar returns different paths in change events depending on + // whether the path provided to the watcher is absolute or relative. If the + // path is absolute, change events will contain absolute paths, and the + // opposite for relative paths. We need to ensure that the paths we provide + // are always absolute to ensure consistency in change events. if (ctx.config.serverEntryPoint) { - toWatch.push(ctx.config.serverEntryPoint); + toWatch.push( + path.resolve(ctx.config.rootDirectory, ctx.config.serverEntryPoint) + ); } - ctx.config.watchPaths?.forEach((watchPath) => { - toWatch.push(watchPath); + toWatch.push(path.resolve(ctx.config.rootDirectory, watchPath)); }); let watcher = chokidar diff --git a/packages/remix-dev/devServer/liveReload.ts b/packages/remix-dev/devServer/liveReload.ts index c09f48887a4..1add646d273 100644 --- a/packages/remix-dev/devServer/liveReload.ts +++ b/packages/remix-dev/devServer/liveReload.ts @@ -7,6 +7,7 @@ import WebSocket from "ws"; import { watch } from "../compiler"; import type { RemixConfig } from "../config"; import { warnOnce } from "../warnOnce"; +import { createFileWatchCache } from "../compiler/fileWatchCache"; const relativePath = (file: string) => path.relative(process.cwd(), file); @@ -37,6 +38,8 @@ export async function liveReload(config: RemixConfig) { broadcast({ type: "LOG", message: _message }); } + let fileWatchCache = createFileWatchCache(); + let hasBuilt = false; let dispose = await watch( { @@ -46,6 +49,7 @@ export async function liveReload(config: RemixConfig) { sourcemap: true, onWarning: warnOnce, }, + fileWatchCache, }, { onBuildStart() { @@ -63,9 +67,11 @@ export async function liveReload(config: RemixConfig) { }, onFileChanged(file) { log(`File changed: ${relativePath(file)}`); + fileWatchCache.invalidateFile(file); }, onFileDeleted(file) { log(`File deleted: ${relativePath(file)}`); + fileWatchCache.invalidateFile(file); }, } ); diff --git a/packages/remix-dev/devServer_unstable/index.ts b/packages/remix-dev/devServer_unstable/index.ts index 1bbf1de6aee..df572d07c2b 100644 --- a/packages/remix-dev/devServer_unstable/index.ts +++ b/packages/remix-dev/devServer_unstable/index.ts @@ -10,6 +10,7 @@ import express from "express"; import * as Channel from "../channel"; import { type Manifest } from "../manifest"; import * as Compiler from "../compiler"; +import { createFileWatchCache } from "../compiler/fileWatchCache"; import { type RemixConfig } from "../config"; import { loadEnv } from "./env"; import * as Socket from "./socket"; @@ -170,6 +171,8 @@ export let serve = async ( return newAppServer; }; + let fileWatchCache = createFileWatchCache(); + let dispose = await Compiler.watch( { config: initialConfig, @@ -179,6 +182,7 @@ export let serve = async ( onWarning: warnOnce, devOrigin: origin, }, + fileWatchCache, }, { onBuildStart: async (ctx) => { @@ -246,10 +250,14 @@ export let serve = async ( }, onFileCreated: (file) => websocket.log(`File created: ${relativePath(file)}`), - onFileChanged: (file) => - websocket.log(`File changed: ${relativePath(file)}`), - onFileDeleted: (file) => - websocket.log(`File deleted: ${relativePath(file)}`), + onFileChanged: (file) => { + websocket.log(`File changed: ${relativePath(file)}`); + fileWatchCache.invalidateFile(file); + }, + onFileDeleted: (file) => { + websocket.log(`File deleted: ${relativePath(file)}`); + fileWatchCache.invalidateFile(file); + }, } ); diff --git a/packages/remix-dev/package.json b/packages/remix-dev/package.json index 952bab9f446..ef1bc7f817d 100644 --- a/packages/remix-dev/package.json +++ b/packages/remix-dev/package.json @@ -53,6 +53,7 @@ "minimatch": "^9.0.0", "node-fetch": "^2.6.9", "ora": "^5.4.1", + "picomatch": "^2.3.1", "postcss": "^8.4.19", "postcss-discard-duplicates": "^5.1.0", "postcss-load-config": "^4.0.1", @@ -80,6 +81,7 @@ "@types/lodash.debounce": "^4.0.6", "@types/node-fetch": "^2.5.7", "@types/npmcli__package-json": "^2.0.0", + "@types/picomatch": "^2.3.0", "@types/shelljs": "^0.8.11", "@types/tar-fs": "^2.0.1", "@types/ws": "^7.4.1", diff --git a/yarn.lock b/yarn.lock index c0d4a6ba4a7..91474687b92 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3249,6 +3249,11 @@ resolved "https://registry.npmjs.org/@types/npmcli__package-json/-/npmcli__package-json-2.0.0.tgz" integrity sha512-8/+/ZIuh9aZjW8QIrnpRwUqUoEXCQePzH03WRZSx+3RtBZhQI4ytVRVsfhMjYtxZpJiepXLW27WKPfDl2o/i8Q== +"@types/picomatch@^2.3.0": + version "2.3.0" + resolved "https://registry.npmjs.org/@types/picomatch/-/picomatch-2.3.0.tgz#75db5e75a713c5a83d5b76780c3da84a82806003" + integrity sha512-O397rnSS9iQI4OirieAtsDqvCj4+3eY1J+EPdNTKuHuRWIfUoGyzX294o8C4KJYaLqgSrd2o60c5EqCU8Zv02g== + "@types/prettier@^2.1.5": version "2.4.4" resolved "https://registry.npmjs.org/@types/prettier/-/prettier-2.4.4.tgz" @@ -10992,6 +10997,15 @@ postcss-import@^14.1.0: read-cache "^1.0.0" resolve "^1.1.7" +postcss-import@^15.1.0: + version "15.1.0" + resolved "https://registry.npmjs.org/postcss-import/-/postcss-import-15.1.0.tgz#41c64ed8cc0e23735a9698b3249ffdbf704adc70" + integrity sha512-hpr+J05B2FVYUAXHeK1YyI267J/dDDhMU6B6civm8hSY1jYJnBXxzKDKDswzJmtLHryrjhnDjqqp/49t8FALew== + dependencies: + postcss-value-parser "^4.0.0" + read-cache "^1.0.0" + resolve "^1.1.7" + postcss-js@^4.0.0: version "4.0.0" resolved "https://registry.npmjs.org/postcss-js/-/postcss-js-4.0.0.tgz"