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

[KingSora]

Closes: #1153

• Docs
• Tests

Testing Strategy:
I've tested it like:

• create demo remix app
• add css file with css assets (such as .svgs, .pngs, etc.)
• run remix in watch or build mode
• verify whether all assets are in the _assets folder (server and client)
• verify imported css paths are correct on server and client

Note: I have only tested relative url imports (background: url('./relative');), the following are untested:

• absolute url imports background: url('/absolute');
• data imports background: url('data:image/gif;base64,...');
• url imports background: url('http://...');
• idimports background: url('#myid');

[changeset-bot]

🦋 Changeset detected

Latest commit: f77eeca

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages

Name	Type
remix	Patch
@remix-run/dev	Patch
create-remix	Patch
@remix-run/architect	Patch
@remix-run/cloudflare	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
@remix-run/deno	Patch
@remix-run/eslint-config	Patch
@remix-run/express	Patch
@remix-run/netlify	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/serve	Patch
@remix-run/server-runtime	Patch
@remix-run/vercel	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[remix-cla-bot]

Hi @KingSora,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at hello@remix.run.

Thanks!

- The Remix team

[remix-cla-bot]

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

[mcansh]

first off, thanks a ton for looking into this!

Note: I have only tested relative url imports (background: url('./relative');), the following are untested:

• absolute url imports background: url('/absolute');
• data imports background: url('data:image/gif;base64,...');
• url imports background: url('http://...');
• idimports background: url('#myid');

looks like data urls, regular urls and id "imports" work, but an absolute path doesn't as a result of using the "css loader" it appears

[KingSora]

@mcansh thanks for taking a look! :)

I've pushed an update fixing the absolute path url stuff..
I also wanted to add some tests, but I can't really figure out a good test strategy.. could you help me with that?

[mcansh]

I also wanted to add some tests, but I can't really figure out a good test strategy.. could you help me with that?

I've been thinking about this a bit, as far as making sure files from node_modules are copied, i think that should be relatively straight forward by running a build, getting the list of files/hashes and writing an assertion, but making sure everything else that exists in the css space still works seems a bit trickier to me

[KingSora]

@mcansh I've now improved the code by removing all the path.join logic and rely solely on what esbuild does with its path resoluton

Edit: Also added watchFiles so changes to the asset files are noticed in watch mode.

[mcansh]

this is looking great @KingSora, one thing i noticed when playing around with it today is that in order to use a fontforge package with fonts and css, you need to explicitly add the index.css to the end of the import (e.g @fontsource/aguafina-script/index.css instead of @fontsource/aguafina-script), I imagine that's due to the filter looking for only css files, but could be wrong and I'm not sure what the perf differences are if we filter for all files and return early if not css.

basic change to use filter: /.*/ and if (!args.path.endsWith(".css")) return; shows about ~50ms which could add up with a lot of files

[KingSora]

@mcansh thanks for the review! Are you sure this is due to this plugin? I thought in the onLoad hook esbuild is using already the resolved path and thus the /\.css$/ regex should match even in your case.

On a side note, I could also add caching here, so that this process is even faster

[mcansh]

Are you sure this is due to this plugin?

I think so? altering the filter to have everything go through it allows for import fontStylesheetUrl from "@fontsource/aguafina-script"; to work, though typescript isn't happy with it which is expected as they don't ship a declaration file

edit: actually building is fine with the changed filter, but dev doesn't work with it..

[KingSora]

@mcansh whats the error you get in dev mode if you change the filter regex and check the file extension inside the onLoad hook?

[mcansh]

this happens when using either filter for onLoad (seems like esbuild is attempting to load it as js even though "main" is pointed to index.css) - which may be fine with some docs

> dev
> run-p "dev:*"


> dev:server
> cross-env NODE_ENV=development node --inspect --require ./node_modules/dotenv/config ./server.js


> dev:css
> cross-env NODE_ENV=development npm run generate:css -- --watch


> dev:remix
> cross-env NODE_ENV=development node ./node_modules/@remix-run/dev/dist/cli watch

Debugger listening on ws://127.0.0.1:9229/9caea8f2-23ee-4104-9d45-c25ef52e7068
For help, see: https://nodejs.org/en/docs/inspector
/Users/logan/Developer/github.com/remix-run/kingsora-remix/playground/playground-1665525447492/node_modules/@fontsource/aguafina-script/index.css:2
@font-face {
^

SyntaxError: Invalid or unexpected token
    at Object.compileFunction (node:vm:360:18)
    at wrapSafe (node:internal/modules/cjs/loader:1055:15)
    at Module._compile (node:internal/modules/cjs/loader:1090:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10)
    at Module.load (node:internal/modules/cjs/loader:1004:32)
    at Function.Module._load (node:internal/modules/cjs/loader:839:12)
    at Module.require (node:internal/modules/cjs/loader:1028:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at Object.<anonymous> (/Users/logan/Developer/github.com/remix-run/kingsora-remix/playground/playground-1665525447492/build/index.js:75:126)
    at Module._compile (node:internal/modules/cjs/loader:1126:14)

> generate:css
> tailwindcss -o ./app/styles/tailwind.css --watch

ERROR: "dev:server" exited with 1.

[KingSora]

@mcansh would you say this functionality is crucial for this PR to be merged? If so, I can try to fix it in a "hacky" way. If not I would open a issue in the esbuild repo, and we could improve on that behavior in a follow up PR... what do you think?

[mcansh]

@mcansh would you say this functionality is crucial for this PR to be merged? If so, I can try to fix it in a "hacky" way. If not I would open a issue in the esbuild repo, and we could improve on that behavior in a follow up PR... what do you think?

personally, I think it's fine as is if we add something about it to https://remix.run/pages/gotchas

suggestion was committed

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-a0823ed-20221015 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

[KingSora]

@mcansh I've created an issue in the esbuild repo, and the answer is: evanw/esbuild#2616

So as stated in my answer here: #4130 (comment) I believe everything should be fine with this plugin, are you sure other plugins aren't changing the path to the entry file?
The yarnPnpPlugin looks sus and shouldn't be needed anymore since esbuild 0.15.0.

Also here a minimal example: https://stackblitz.com/edit/node-2fnbwh?file=index.mjs

[mcansh]

thanks for the exploration @KingSora, we're hesitant to bump esbuild currently as there's a few things in there we need to test out (native pnp support included), looking into it soon

just cut an experimental release with esbuild 0.15.11 0.0.0-experimental-aed5bfbf7 and it still seems to require the full import, going to take a look at our other plugins when i get a chance

[ayuhito]

Just out of curiosity. Are the imported assets then hashed? Would asset preloads then be so simple if it were to be hashed?

[Kim-Chuljoong]

@KingSora hello. I updated the remix and ran into an error like never before.
Some of the libraries I use needed a Buffer, so I'm using remix-esbuild-override. This one worked fine.
But, this plugin also uses esbuild, so there seems to be a conflict.
Here is the error message:

 ✘ [ERROR] [plugin css-file] Build failed with 2 errors:
error: Could not read from file: /***/node_modules/@esbuild-plugins/node-globals-polyfill/_virtual-process-polyfill_.js
node_modules/@esbuild-plugins/node-globals-polyfill/_buffer.js:1:23: ERROR: Could not resolve "_node-buffer-polyfill_.js"

    app/routes/test.jsx:5:19:
      5 │ import styles from "~/styles/test.css";
        ╵ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  This error came from the "onLoad" callback registered here:

    node_modules/@remix-run/dev/dist/compiler/plugins/cssFilePlugin.js:62:14:
      62 │ build$1.onLoad({
         ╵ ~~~~~~~~

    at setup (/***/node_modules/@remix-run/dev/dist/compiler/plugins/cssFilePlugin.js:62:15)

Is there any way to solve this?
Is there an option to disable cssFilePlugin? I think this is the fastest solution 🤔
Or could it be solved through an extra setting in remix-esbuild-override esbuild? If so, what can I do?

[machour]

@Kim-Chuljoong

The best course of action is to open a new discussion in order for your question tu be visible. A comment on a closed PR will likely be dismissed.

[jdnichollsc]

@KingSora hello. I updated the remix and ran into an error like never before. Some of the libraries I use needed a Buffer, so I'm using remix-esbuild-override. This one worked fine. But, this plugin also uses esbuild, so there seems to be a conflict. Here is the error message:

 ✘ [ERROR] [plugin css-file] Build failed with 2 errors:
error: Could not read from file: /***/node_modules/@esbuild-plugins/node-globals-polyfill/_virtual-process-polyfill_.js
node_modules/@esbuild-plugins/node-globals-polyfill/_buffer.js:1:23: ERROR: Could not resolve "_node-buffer-polyfill_.js"

    app/routes/test.jsx:5:19:
      5 │ import styles from "~/styles/test.css";
        ╵ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  This error came from the "onLoad" callback registered here:

    node_modules/@remix-run/dev/dist/compiler/plugins/cssFilePlugin.js:62:14:
      62 │ build$1.onLoad({
         ╵ ~~~~~~~~

    at setup (/***/node_modules/@remix-run/dev/dist/compiler/plugins/cssFilePlugin.js:62:15)

Is there any way to solve this? Is there an option to disable cssFilePlugin? I think this is the fastest solution thinking Or could it be solved through an extra setting in remix-esbuild-override esbuild? If so, what can I do?

I was able to disable cssFilePlugin using the below patch: #4906 (comment)
I'm not sure if there's a better option to avoid having conflicts with this plugin, please let me know in the discussion thread!

[EliBates]

@jdnichollsc @Kim-Chuljoong I've just ran into this problem today, have you found any other solution?

[isNan909]

I also faced the same issue (remix + Cloudflare pages setup) ::

Building Remix app in production mode...
💽 Override esbuild. Your custom config can be used to build for Remix.

✘ [ERROR] [plugin css-file] Build failed with 2 errors:
error: Could not read from file: /../app/node_modules/@esbuild-plugins/node-globals-polyfill/virtual-process-polyfill.js
node_modules/@esbuild-plugins/node-globals-polyfill/_buffer.js:1:23: ERROR: Could not resolve "node-buffer-polyfill.js"

app/routes/_index.tsx:14:19:
  14 │ import styles from "~/assets/index.css";
     ╵                    ~~~~~~~~~~~~~~~~~~~~

Build failed with 1 error:

Please do let us know if we have a fix for this?

How can we temporarily disable cssFilePlugin in this case?

Please do let us know in the discussion please.

[deflexable]

am also having this same issue

SyntaxError: Invalid or unexpected token at Object.compileFunction (node:vm:360:18) at wrapSafe (node:internal/modules/cjs/loader:1055:15) at Module._compile (node:internal/modules/cjs/loader:1090:27) at Object.Module._extensions..js (node:internal/modules/cjs/loader:1180:10) at Module.load (node:internal/modules/cjs/loader:1004:32) at Function.Module._load (node:internal/modules/cjs/loader:839:12) at Module.require (node:internal/modules/cjs/loader:1028:19) at require (node:internal/modules/cjs/helpers:102:18) at Object.<anonymous> (/Users/aoamacsplace/Documents/codes/JambFlex/server/website/examjoint/node_modules/react-summernote-lite/dist/SummernoteLite.js:12:1) at Module._compile (node:internal/modules/cjs/loader:1126:14)