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

CSS Side Effects plugin fails when TypeScript module contains a decorator #5287

Closed
1 task done
ryanwilsonperkin opened this issue Jan 27, 2023 · 4 comments
Closed
1 task done
Assignees
Labels
bug Something isn't working package:css-bundle

Comments

@ryanwilsonperkin
Copy link

What version of Remix are you using?

1.11.1

Are all your remix dependencies & dev-dependencies using the same version?

  • Yes

Steps to Reproduce

Reproduction in repo form: https://github.com/ryanwilsonperkin/remix-css-side-effects-bug/pull/1/files

  1. Enable the CSS Side Effects Plugin
  2. Write a simple CSS file and import it as a side effect to a TypeScript module
  3. Add a decorator in the TypeScript module
  4. See the following error:
✘ [ERROR] [plugin css-side-effects-plugin] This experimental syntax requires enabling one of the following parser plugin(s): "decorators", "decorators-legacy". (9:0)

    browser-route-module:routes/index.tsx?browser:1:24:
      1 │ export { default } from "./routes/index.tsx";
        ╵                         ~~~~~~~~~~~~~~~~~~~~

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

    node_modules/@remix-run/dev/dist/compiler/plugins/cssSideEffectImportsPlugin.js:58:12:
      58 │       build.onLoad({
         ╵             ~~~~~~

    at setup (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/@remix-run/dev/dist/compiler/plugins/cssSideEffectImportsPlugin.js:58:13)
    at handlePlugins (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/esbuild/lib/main.js:1276:21)
    at buildOrServeImpl (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/esbuild/lib/main.js:965:5)
    at Object.buildOrServe (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/esbuild/lib/main.js:773:5)
    at /Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/esbuild/lib/main.js:2112:17
    at new Promise (<anonymous>)
    at Object.build (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/esbuild/lib/main.js:2111:14)
    at Object.build (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/esbuild/lib/main.js:1958:51)
    at appBuildTask (/Users/ryan/src/github.com/ryanwilsonperkin/remix-css-side-effects-bug/node_modules/@remix-run/dev/dist/compiler/compileBrowser.js:164:62)

Expected Behavior

Expect the TypeScript module to be evaluated using the same behaviours that are used during compilation, and so decorators should be acceptable syntax.

Actual Behavior

Here's a PR where I've set up an example Remix repo with a PR that demonstrates the necessary code to reproduce the bug: https://github.com/ryanwilsonperkin/remix-css-side-effects-bug/pull/1/files

What I believe is happening is that when the CSS Side Effects plugin is loaded, it will detect any JS/TS modules that contain the literal string '.css' in their source code and then use Babel to traverse the AST and look for CSS side effect imports. Since Babel is using a different set of features for parsing TypeScript than the compiler that Esbuild is using, it fails when it encounters decorators because it considers them to be experimental syntax, while the Esbuild compiler considers them to be valid.

As a result, any TypeScript file that uses syntax that's not expected by Babel can reproduce this error not just by having a CSS side effect import in it, but even by having the value '.css' in it's source code, such as the following example:

// .css

function noopDecorator(target) {
  return target;
}

@noopDecorator
class C {}

...
@markdalgleish
Copy link
Member

Thanks for the bug report! I've opened a PR fixing this: #5305

@markdalgleish
Copy link
Member

PR has been merged, fix will be in the next release.

@machour machour added the awaiting release This issue has been fixed and will be released soon label Jan 30, 2023
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-52284ab-20230130 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 1.12.0 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@brophdawg11 brophdawg11 removed the awaiting release This issue has been fixed and will be released soon label Apr 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package:css-bundle
Projects
None yet
Development

No branches or pull requests

4 participants