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

feat(remix-dev/compiler): add CSS plugin to esbuild #4130

Merged
merged 26 commits into from
Oct 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6361c2f
create and add css file plugin
KingSora Sep 3, 2022
6f3332e
Update contributors.yml
KingSora Sep 3, 2022
4bc5657
absolute url paths are considered external
KingSora Sep 7, 2022
3b94b24
Merge branch 'dev' of https://github.com/KingSora/remix into dev
KingSora Sep 7, 2022
facd266
add correct sourcemap and minify behavior
KingSora Sep 8, 2022
797022a
improve code
KingSora Sep 12, 2022
54c8b54
add comment why sourcemap false
KingSora Sep 12, 2022
f18bd54
improve code and output
KingSora Sep 12, 2022
000a13a
improve entrypoint search
KingSora Sep 12, 2022
56f8688
Merge branch 'dev' into dev
KingSora Sep 12, 2022
db271ed
better strategy for deletion of duplicates
KingSora Sep 13, 2022
3650a95
Merge branch 'dev' of https://github.com/KingSora/remix into dev
KingSora Sep 13, 2022
1f9ee91
refine process and add watchFiles
KingSora Sep 14, 2022
362250b
pr review
KingSora Sep 16, 2022
a0cb142
Merge branch 'dev' into dev
KingSora Sep 16, 2022
92473d0
add error and warnings
KingSora Sep 28, 2022
47ebf83
Merge branch 'dev' of https://github.com/KingSora/remix into dev
KingSora Sep 28, 2022
a4106a8
Merge branch 'dev' into dev
KingSora Sep 28, 2022
a804d4e
windows slashes fix
KingSora Sep 30, 2022
fc5c271
Merge branch 'dev' of https://github.com/KingSora/remix into dev
KingSora Sep 30, 2022
85d5a9d
fix errors
KingSora Oct 10, 2022
e65fb5e
Merge branch 'dev' into dev
KingSora Oct 10, 2022
0d1344a
chore: couple nit picks
mcansh Oct 14, 2022
c1f5c2b
test: add basic test confirming a font was copied
mcansh Oct 14, 2022
8d95db4
Merge branch 'dev' into dev
mcansh Oct 14, 2022
f77eeca
Create large-colts-drop.md
mcansh Oct 14, 2022
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
8 changes: 8 additions & 0 deletions .changeset/large-colts-drop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"remix": patch
"@remix-run/dev": patch
---

add CSS plugin to `esbuild` so that any assets in css files are also copied (and hashed) to the `assetsBuildDirectory`

currently if you import a css file that has `background: url('./relative.png');` the `relative.png` file is not copied to the build directory, which is a problem when dealing with npm packages that have css files with font files in them like fontsource
1 change: 1 addition & 0 deletions contributors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@
- kilian
- kiliman
- kimdontdoit
- KingSora
- klauspaiva
- knowler
- konradkalemba
Expand Down
44 changes: 42 additions & 2 deletions integration/compiler-test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import path from "path";
import fs from "fs/promises";
import fse from "fs-extra";
import { test, expect } from "@playwright/test";
import { PassThrough } from "stream";

Expand All @@ -9,6 +9,7 @@ import {
js,
json,
createFixtureProject,
css,
} from "./helpers/create-fixture";
import type { Fixture, AppFixture } from "./helpers/create-fixture";
import { PlaywrightFixture } from "./helpers/playwright-fixture";
Expand Down Expand Up @@ -89,6 +90,18 @@ test.describe("compiler", () => {
return <div id="package-with-submodule">{submodule()}</div>;
}
`,
"app/routes/css.jsx": js`
import stylesUrl from "@org/css/index.css";

export function links() {
return [{ rel: "stylesheet", href: stylesUrl }]
}

export default function PackageWithSubModule() {
return <div id="package-with-submodule">{submodule()}</div>;
}
`,

"remix.config.js": js`
let { getDependenciesToBundle } = require("@remix-run/dev");
module.exports = {
Expand Down Expand Up @@ -156,6 +169,22 @@ test.describe("compiler", () => {
return "package-with-submodule";
}
`,
"node_modules/@org/css/package.json": json({
name: "@org/css",
version: "1.0.0",
main: "index.css",
}),
"node_modules/@org/css/font.woff2": "font",
"node_modules/@org/css/index.css": css`
body {
background: red;
}

@font-face {
font-family: "MyFont";
src: url("./font.woff2");
}
`,
},
});

Expand Down Expand Up @@ -266,6 +295,17 @@ test.describe("compiler", () => {
);
});

test("copies imports in css files to assetsBuildDirectory", async () => {
let buildDir = path.join(fixture.projectDir, "public", "build", "_assets");
let files = await fse.readdir(buildDir);
expect(files).toHaveLength(2);

let cssFile = files.find((file) => file.match(/index-[a-z0-9]{8}\.css/i));
let fontFile = files.find((file) => file.match(/font-[a-z0-9]{8}\.woff2/i));
expect(cssFile).toBeTruthy();
expect(fontFile).toBeTruthy();
});

// TODO: remove this when we get rid of that feature.
test("magic imports still works", async () => {
let magicExportsForNode = [
Expand Down Expand Up @@ -314,7 +354,7 @@ test.describe("compiler", () => {
"useSubmit",
"useTransition",
];
let magicRemix = await fs.readFile(
let magicRemix = await fse.readFile(
path.resolve(fixture.projectDir, "node_modules/remix/dist/index.js"),
"utf8"
);
Expand Down
5 changes: 4 additions & 1 deletion packages/remix-dev/compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ import { serverAssetsManifestPlugin } from "./compiler/plugins/serverAssetsManif
import { serverBareModulesPlugin } from "./compiler/plugins/serverBareModulesPlugin";
import { serverEntryModulePlugin } from "./compiler/plugins/serverEntryModulePlugin";
import { serverRouteModulesPlugin } from "./compiler/plugins/serverRouteModulesPlugin";
import { cssFilePlugin } from "./compiler/plugins/cssFilePlugin";
import { writeFileSafe } from "./compiler/utils/fs";
import { urlImportsPlugin } from "./compiler/plugins/urlImportsPlugin";

interface BuildConfig {
export interface BuildConfig {
mode: BuildMode;
target: BuildTarget;
sourcemap: boolean;
Expand Down Expand Up @@ -347,6 +348,7 @@ async function createBrowserBuild(
}

let plugins = [
cssFilePlugin(options),
urlImportsPlugin(),
mdxPlugin(config),
browserRouteModulesPlugin(config, /\?browser$/),
Expand Down Expand Up @@ -415,6 +417,7 @@ function createServerBuild(
let isDenoRuntime = config.serverBuildTarget === "deno";

let plugins: esbuild.Plugin[] = [
cssFilePlugin(options),
urlImportsPlugin(),
mdxPlugin(config),
emptyModulesPlugin(config, /\.client(\.[jt]sx?)?$/),
Expand Down
113 changes: 113 additions & 0 deletions packages/remix-dev/compiler/plugins/cssFilePlugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import * as path from "path";
import * as fse from "fs-extra";
import esbuild from "esbuild";

import { BuildMode } from "../../build";
import type { BuildConfig } from "../../compiler";
import invariant from "../../invariant";

const isExtendedLengthPath = /^\\\\\?\\/;

function normalizePathSlashes(p: string) {
return isExtendedLengthPath.test(p) ? p : p.replace(/\\/g, "/");
}

/**
* 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(
buildConfig: Pick<Partial<BuildConfig>, "mode">
): esbuild.Plugin {
return {
name: "css-file",

async setup(build) {
let buildOps = build.initialOptions;

build.onLoad({ filter: /\.css$/ }, async (args) => {
let { outfile, outdir, assetNames } = buildOps;
let { metafile, outputFiles, warnings, errors } = await esbuild.build({
...buildOps,
minify: buildConfig.mode === BuildMode.Production,
minifySyntax: true,
metafile: true,
write: false,
sourcemap: false,
incremental: false,
splitting: false,
stdin: undefined,
outfile: undefined,
outdir: outfile ? path.dirname(outfile) : outdir,
entryNames: assetNames,
entryPoints: [args.path],
loader: {
...buildOps.loader,
".css": "css",
},
// this plugin treats absolute paths in 'url()' css rules as external to prevent breaking changes
plugins: [
{
name: "resolve-absolute",
async setup(build) {
build.onResolve({ filter: /.*/ }, async (args) => {
let { kind, path: resolvePath } = args;
if (kind === "url-token" && path.isAbsolute(resolvePath)) {
return {
path: resolvePath,
external: true,
};
}
});
},
},
],
});

if (errors && errors.length) {
return { errors };
}

invariant(metafile, "metafile is missing");
let { outputs } = metafile;
let entry = Object.keys(outputs).find((out) => outputs[out].entryPoint);
invariant(entry, "entry point not found");

let normalizedEntry = normalizePathSlashes(entry);
let entryFile = outputFiles.find((file) => {
return normalizePathSlashes(file.path).endsWith(normalizedEntry);
});

invariant(entryFile, "entry file not found");

let outputFilesWithoutEntry = outputFiles.filter(
(file) => file !== entryFile
);

// write all assets
await Promise.all(
outputFilesWithoutEntry.map(({ path: filepath, contents }) =>
fse.outputFile(filepath, contents)
)
);

return {
contents: entryFile.contents,
loader: "file",
// add all css assets to watchFiles
watchFiles: Object.values(outputs).reduce<string[]>(
(arr, { inputs }) => {
let resolvedInputs = Object.keys(inputs).map((input) => {
return path.resolve(input);
});
arr.push(...resolvedInputs);
return arr;
},
[]
),
warnings,
};
});
},
};
}