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: handle CSS bundle updates during HMR #5823

Merged
merged 25 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
dadde5b
fix: handle CSS bundle updates during HMR
markdalgleish Mar 16, 2023
ec86cbb
Add extra wait to test
markdalgleish Mar 16, 2023
8cc612d
Move CSS update into separate step in test
markdalgleish Mar 16, 2023
5507895
Use styles on original test index route
markdalgleish Mar 16, 2023
0000e5a
Merge branch 'dev' into markdalgleish/css-bundle-hmr
markdalgleish Mar 16, 2023
d933a6a
Remove unused test ID
markdalgleish Mar 16, 2023
03287a0
Temporarily roll back CSS HMR test
markdalgleish Mar 16, 2023
450c400
Add static bundled CSS to HMR test
markdalgleish Mar 17, 2023
01d68fd
Debug test
markdalgleish Mar 17, 2023
572dc58
Roll back app build check
markdalgleish Mar 17, 2023
7bd098d
Roll back console debug
markdalgleish Mar 17, 2023
53e42ae
Temporarily move test CSS to static file
markdalgleish Mar 17, 2023
cad36b2
Reinstate app build check
markdalgleish Mar 17, 2023
9d66c98
Remove CSS usage from test
markdalgleish Mar 17, 2023
80dec78
Add css-bundle to test deps
markdalgleish Mar 17, 2023
4bbedf1
Fix CSS build, convert promise to channel
markdalgleish Mar 17, 2023
8eebb22
Revert to original CSS bundle HMR test
markdalgleish Mar 17, 2023
633fa37
Only apply CSS bundle update plugin for HMR
markdalgleish Mar 17, 2023
28351fe
Fix type error
markdalgleish Mar 17, 2023
9840d20
Remove redundant channel read alias
markdalgleish Mar 17, 2023
8c77eed
Merge branch 'dev' into markdalgleish/css-bundle-hmr
markdalgleish Mar 17, 2023
b60db44
Move all CSS href channel access into CSS build
markdalgleish Mar 19, 2023
560ee6a
Merge branch 'dev' into markdalgleish/css-bundle-hmr
markdalgleish Mar 19, 2023
60a4910
Refactor
markdalgleish Mar 19, 2023
d089f9c
Clean up
markdalgleish Mar 20, 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
6 changes: 6 additions & 0 deletions .changeset/css-bundle-hmr.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@remix-run/css-bundle": patch
"@remix-run/dev": patch
---

Ensure changes to CSS inserted via `@remix-run/css-bundle` are picked up during HMR
24 changes: 23 additions & 1 deletion integration/hmr-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ let fixture = (options: { port: number; appServerPort: number }) => ({
port: options.port,
appServerPort: options.appServerPort,
},
unstable_cssModules: true,
unstable_tailwind: true,
v2_routeConvention: true,
v2_errorBoundary: true,
Expand All @@ -26,6 +27,7 @@ let fixture = (options: { port: number; appServerPort: number }) => ({
"dev:app": `cross-env NODE_ENV=development nodemon --watch build/ ./server.js`,
},
dependencies: {
"@remix-run/css-bundle": "0.0.0-local-version",
"@remix-run/node": "0.0.0-local-version",
"@remix-run/react": "0.0.0-local-version",
"cross-env": "0.0.0-local-version",
Expand Down Expand Up @@ -90,15 +92,23 @@ let fixture = (options: { port: number; appServerPort: number }) => ({
@tailwind utilities;
`,

"app/styles.module.css": css`
.test {
color: initial;
}
`,

"app/root.tsx": js`
import type { LinksFunction } from "@remix-run/node";
import { Link, Links, LiveReload, Meta, Outlet, Scripts } from "@remix-run/react";
import { cssBundleHref } from "@remix-run/css-bundle";

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

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

export default function Root() {
Expand Down Expand Up @@ -246,15 +256,26 @@ 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 cssModulePath = path.join(projectDir, "app", "styles.module.css");
let originalCssModule = fs.readFileSync(cssModulePath, "utf8");

// make content and style changed to index route
let newCssModule = `
.test {
background: black;
color: white;
}
`;
fs.writeFileSync(cssModulePath, newCssModule);

let newIndex = `
import { useLoaderData } from "@remix-run/react";
import styles from "~/styles.module.css";
export default function Index() {
const t = useLoaderData();
return (
<main>
<h1 className="text-white bg-black">Changed</h1>
<h1 className={styles.test}>Changed</h1>
</main>
)
}
Expand All @@ -274,6 +295,7 @@ test("HMR", async ({ page }) => {

// undo change
fs.writeFileSync(indexPath, originalIndex);
fs.writeFileSync(cssModulePath, originalCssModule);
await page.getByText("Index Title").waitFor({ timeout: 2000 });
expect(await page.getByLabel("Root Input").inputValue()).toBe("asdfasdf");
await page.waitForSelector(`#root-counter:has-text("inc 1")`);
Expand Down
10 changes: 9 additions & 1 deletion packages/remix-css-bundle/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,12 @@ import type { AssetsManifest } from "@remix-run/dev/assets-manifest";

let assetsManifest: AssetsManifest = (window as any).__remixManifest;

export const cssBundleHref = assetsManifest.cssBundleHref;
declare const __INJECT_CSS_BUNDLE_HREF__: string | undefined;

// Injected by `cssBundleUpdatePlugin` on rebuilds
let updatedHref: string | undefined =
typeof __INJECT_CSS_BUNDLE_HREF__ === "string"
? __INJECT_CSS_BUNDLE_HREF__
: undefined;

export const cssBundleHref = updatedHref || assetsManifest.cssBundleHref;
6 changes: 2 additions & 4 deletions packages/remix-dev/compiler/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,12 @@ export interface AssetsManifest {
export async function createAssetsManifest({
config,
metafile,
cssBundlePath,
cssBundleHref,
hmr,
}: {
config: RemixConfig;
metafile: esbuild.Metafile;
cssBundlePath?: string;
cssBundleHref?: string;
hmr?: AssetsManifest["hmr"];
}): Promise<AssetsManifest> {
function resolveUrl(outputPath: string): string {
Expand Down Expand Up @@ -126,8 +126,6 @@ export async function createAssetsManifest({
JSON.stringify({ entry, routes, hmrRoutes: hmr?.routes })
).slice(0, 8);

let cssBundleHref = cssBundlePath ? resolveUrl(cssBundlePath) : undefined;

return { version, entry, routes, cssBundleHref, hmr };
}

Expand Down
178 changes: 105 additions & 73 deletions packages/remix-dev/compiler/compileBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import { NodeModulesPolyfillPlugin } from "@esbuild-plugins/node-modules-polyfil
import postcss from "postcss";
import postcssDiscardDuplicates from "postcss-discard-duplicates";

import type { WriteChannel } from "../channel";
import type { Channel, WriteChannel } from "../channel";
import { createChannel } from "../channel";
import type { RemixConfig } from "../config";
import type { AssetsManifest } from "./assets";
import { createAssetsManifest } from "./assets";
Expand All @@ -20,6 +21,7 @@ import { deprecatedRemixPackagePlugin } from "./plugins/deprecatedRemixPackagePl
import { emptyModulesPlugin } from "./plugins/emptyModulesPlugin";
import { mdxPlugin } from "./plugins/mdx";
import { urlImportsPlugin } from "./plugins/urlImportsPlugin";
import { cssBundleUpdatePlugin } from "./plugins/cssBundleUpdatePlugin";
import { cssModulesPlugin } from "./plugins/cssModulesPlugin";
import { cssSideEffectImportsPlugin } from "./plugins/cssSideEffectImportsPlugin";
import { vanillaExtractPlugin } from "./plugins/vanillaExtractPlugin";
Expand Down Expand Up @@ -78,6 +80,12 @@ const isCssBundlingEnabled = (config: RemixConfig): boolean =>
config.future.unstable_cssSideEffectImports ||
config.future.unstable_vanillaExtract
);

let cssBundleHrefChannel: Channel<string | undefined>;

// This function gives esbuild access to the latest channel value on rebuilds
let getCssBundleHref = () => cssBundleHrefChannel.read();

const createEsbuildConfig = (
build: "app" | "css",
config: RemixConfig,
Expand Down Expand Up @@ -131,7 +139,7 @@ const createEsbuildConfig = (
NodeModulesPolyfillPlugin(),
].filter(isNotNull);

if (mode === "development" && config.future.unstable_dev) {
if (build === "app" && mode === "development" && config.future.unstable_dev) {
// TODO prebundle deps instead of chunking just these ones
let isolateChunks = [
require.resolve("react"),
Expand All @@ -149,6 +157,10 @@ const createEsbuildConfig = (
};

plugins.push(hmrPlugin({ remixConfig: config }));

if (isCssBundlingEnabled(config)) {
plugins.push(cssBundleUpdatePlugin({ getCssBundleHref }));
}
}

return {
Expand Down Expand Up @@ -234,82 +246,102 @@ export const createBrowserCompiler = (
return;
}

// The types aren't great when combining write: false and incremental: true
// so we need to assert that it's an incremental build
cssCompiler = (await (!cssCompiler
? esbuild.build({
...createEsbuildConfig("css", remixConfig, options, onLoader),
metafile: true,
incremental: true,
write: false,
})
: cssCompiler.rebuild())) as esbuild.BuildIncremental;

invariant(
cssCompiler.metafile,
"Expected CSS compiler metafile to be defined. This is likely a bug in Remix. Please open an issue at https://github.com/remix-run/remix/issues/new"
);

let outputFiles = cssCompiler.outputFiles || [];

let isCssBundleFile = (
outputFile: esbuild.OutputFile,
extension: ".css" | ".css.map"
): boolean => {
return (
path.dirname(outputFile.path) === remixConfig.assetsBuildDirectory &&
path.basename(outputFile.path).startsWith("css-bundle") &&
outputFile.path.endsWith(extension)
try {
// The types aren't great when combining write: false and incremental: true
// so we need to assert that it's an incremental build
cssCompiler = (await (!cssCompiler
? esbuild.build({
...createEsbuildConfig("css", remixConfig, options, onLoader),
metafile: true,
incremental: true,
write: false,
})
: cssCompiler.rebuild())) as esbuild.BuildIncremental;

invariant(
cssCompiler.metafile,
"Expected CSS compiler metafile to be defined. This is likely a bug in Remix. Please open an issue at https://github.com/remix-run/remix/issues/new"
);
};

let cssBundleFile = outputFiles.find((outputFile) =>
isCssBundleFile(outputFile, ".css")
);
let outputFiles = cssCompiler.outputFiles || [];

let isCssBundleFile = (
outputFile: esbuild.OutputFile,
extension: ".css" | ".css.map"
): boolean => {
return (
path.dirname(outputFile.path) ===
remixConfig.assetsBuildDirectory &&
path.basename(outputFile.path).startsWith("css-bundle") &&
outputFile.path.endsWith(extension)
);
};

let cssBundleFile = outputFiles.find((outputFile) =>
isCssBundleFile(outputFile, ".css")
);

if (!cssBundleFile) {
return;
if (!cssBundleFile) {
cssBundleHrefChannel.write(undefined);
return;
}

let cssBundlePath = cssBundleFile.path;

let cssBundleHref =
remixConfig.publicPath +
path.relative(
remixConfig.assetsBuildDirectory,
path.resolve(cssBundlePath)
);

cssBundleHrefChannel.write(cssBundleHref);

let { css, map } = await postcss([
// We need to discard duplicate rules since "composes"
// in CSS Modules can result in duplicate styles
postcssDiscardDuplicates(),
]).process(cssBundleFile.text, {
from: cssBundlePath,
to: cssBundlePath,
map: options.sourcemap && {
prev: outputFiles.find((outputFile) =>
isCssBundleFile(outputFile, ".css.map")
)?.text,
inline: false,
annotation: false,
sourcesContent: true,
},
});

await fse.ensureDir(path.dirname(cssBundlePath));

await Promise.all([
fse.writeFile(cssBundlePath, css),
options.mode !== "production" && map
? fse.writeFile(`${cssBundlePath}.map`, map.toString()) // Write our updated source map rather than esbuild's
: null,
...outputFiles
.filter((outputFile) => !/\.(css|js|map)$/.test(outputFile.path))
.map(async (asset) => {
await fse.ensureDir(path.dirname(asset.path));
await fse.writeFile(asset.path, asset.contents);
}),
]);

return cssBundleHref;
} catch (error) {
cssBundleHrefChannel.write(undefined);
throw error;
}

let cssBundlePath = cssBundleFile.path;

let { css, map } = await postcss([
// We need to discard duplicate rules since "composes"
// in CSS Modules can result in duplicate styles
postcssDiscardDuplicates(),
]).process(cssBundleFile.text, {
from: cssBundlePath,
to: cssBundlePath,
map: options.sourcemap && {
prev: outputFiles.find((outputFile) =>
isCssBundleFile(outputFile, ".css.map")
)?.text,
inline: false,
annotation: false,
sourcesContent: true,
},
});

await fse.ensureDir(path.dirname(cssBundlePath));

await Promise.all([
fse.writeFile(cssBundlePath, css),
options.mode !== "production" && map
? fse.writeFile(`${cssBundlePath}.map`, map.toString()) // Write our updated source map rather than esbuild's
: null,
...outputFiles
.filter((outputFile) => !/\.(css|js|map)$/.test(outputFile.path))
.map(async (asset) => {
await fse.ensureDir(path.dirname(asset.path));
await fse.writeFile(asset.path, asset.contents);
}),
]);

// Return the CSS bundle path so we can use it to generate the manifest
return cssBundlePath;
};

let [cssBundlePath, metafile] = await Promise.all([
// Reset the channel to co-ordinate the CSS and app builds
if (isCssBundlingEnabled(remixConfig)) {
cssBundleHrefChannel = createChannel();
}

let [cssBundleHref, metafile] = await Promise.all([
cssBuildTask(),
appBuildTask(),
]);
Expand All @@ -336,7 +368,7 @@ export const createBrowserCompiler = (
let manifest = await createAssetsManifest({
config: remixConfig,
metafile: appCompiler.metafile!,
cssBundlePath,
cssBundleHref,
hmr,
});
await writeAssetsManifest(remixConfig, manifest);
Expand Down
Loading