next.js icon indicating copy to clipboard operation
next.js copied to clipboard

Turbopack does not compile CSS nesting for styled-jsx

Open vpontis opened this issue 2 years ago • 7 comments

Link to the code that reproduces this issue

https://github.com/vpontis/nextjs-14-turbo-styled-jsx

To Reproduce

The Turbopack docs it says that it supports PostCSS nested which allows you to nest CSS selectors.

This would be very useful for us since we are currently using Sass just to be able to nest selectors. We would like to move off Sass / Babel and use Turbo.

But I think Turbopack doesn't apply the PostCSS plugins to the CSS generated by Styled JSX.

For example, the following code:

export default function Home() {
  return (
    <>
      <div className={'container'}>
        container (should be blue)

        <div className="inner">
          container + inner (should be yellow)
        </div>
      </div>

      <style jsx>{`
        .container {
          color: blue;
          padding: 3rem;

          .inn {
            &er {
              color: yellow;
            }
          }
        }
      `}</style>
    </>
  )
}

Gives me this CSS:

.container {
    color: blue;
    padding: 3rem;

    .inn {
        &er {
            color: yellow
        }
    }
}

Where I would expect:

.container {
    color: blue;
    padding: 3rem;
}

.container .inner {
    color: yellow;
}

Current vs. Expected behavior

CSS nesting should be compiled but it's not.

Verify canary release

  • [X] I verified that the issue exists in the latest Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: arm64
  Version: Darwin Kernel Version 23.1.0: Mon Oct  9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000
Binaries:
  Node: 18.16.1
  npm: 9.5.1
  Yarn: 1.22.19
  pnpm: 8.10.0
Relevant Packages:
  next: 14.0.0
  eslint-config-next: 14.0.0
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.2.2
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

SWC transpilation, Turbopack (--turbo)

Additional context

This would be super super helpful to us to get working :). Happy to look into this if you'd like to point me in the right direction :)

PACK-2106

vpontis avatar Oct 29 '23 18:10 vpontis

Investigation

swc-project/plugins/styled-jsx is shared among the Wasm plugin and styled-jsx implementation of turbopack, but it has no code to run something on CSS

kdy1 avatar Oct 30 '23 03:10 kdy1

Investigation

swc-project/plugins/styled-jsx is shared among the Wasm plugin and styled-jsx implementation of turbopack, but it has no code to run something on CSS

Ah gotcha — does that mean there's no ability to process the CSS in styled-jsx with a transform like the PostCSS above? Could / would something like that be added?

If so, do you have any recommendation on what we should do in order to get our codebase (lu.ma) ready for Turbopack / SWC?

We extensively use SCSS for CSS nesting (via Babel plugin) in Styled JSX — should we try and migrate off of CSS nesting entirely?

Thanks for looking into this @kdy1!

vpontis avatar Oct 30 '23 03:10 vpontis

Hi @kdy1 just wanted to follow up what you are thinking here.

Are you planning on adding a post processor for nested CSS into SWC / Turbopack? Any advice you could share would be awesome. 🙏

vpontis avatar Dec 19 '23 04:12 vpontis

@vpontis how do you feel about native CSS nesting? Is that an option to consider?

https://caniuse.com/css-nesting

82% browser adoption at the moment. If you need syntax lowering, that's possible as well.

leerob avatar Jan 10 '24 18:01 leerob

@vpontis how do you feel about native CSS nesting? Is that an option to consider?

caniuse.com/css-nesting

82% browser adoption at the moment. If you need syntax lowering, that's possible as well.

Hi @leerob thanks for getting back to us here.

We can switch to native CSS nesting syntax in our code but we will need to compile it to a format that supports more browsers. This is for lu.ma and we want to make sure it's accessible for as many people as possible. We have dropped IE 11, but we can't drop ~ 20% of browsers (esp when it includes iOS Safari 16).

Do you have any other suggestions of what we can do here? My idea was to do a PostCSS phase in SWC / Turbopack if that's possible...

vpontis avatar Jan 10 '24 19:01 vpontis

Thank you @kdy1! I'll try to test this out later today 👍. Really appreciate it.

vpontis avatar Jan 24 '24 15:01 vpontis

Opening as this is reverted. https://github.com/vercel/turbo/pull/7163

kdy1 avatar Jan 30 '24 12:01 kdy1

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

github-actions[bot] avatar Feb 14 '24 12:02 github-actions[bot]

As turbopack is currently dev-mode-only, you need to configure the styled-jsx/babel plugin with the CSS plugins to apply it to the production build. https://github.com/vercel/styled-jsx#css-preprocessing-via-plugins

kdy1 avatar Mar 06 '24 12:03 kdy1

Hi @kdy1, could you share a bit more information about how to get this working? I've been struggling with it for a few weeks.

I made a small repo that shows how this currently works for me — https://github.com/luma-team/nextjs-swc-sass-test

I am running it in dev and I see that the CSS is still nested. I have postcss-nested installed in package.json. Is that what I need? Any instructions would be really helpful.

Screenshot from my demo page:

CleanShot 2024-03-06 at 11 05 29@2x

vpontis avatar Mar 06 '24 16:03 vpontis

You need to pass the css plugin names to stlyed jsx babel plugin

kdy1 avatar Mar 07 '24 03:03 kdy1

@kdy1 can you share a little more? I'm still lost.

Are you suggesting that I need to import the styled-jsx-plugin-postcss to get this working on both dev and production?

I would expect Turbopack to compile the CSS on dev since I am running next dev --turbo but it is not compiling on dev. How should I go about compiling nested CSS on dev with Turbopack?


I created a .babelrc like this:

{
  "plugins": [
    [
      "styled-jsx/babel",
      {
        "plugins": ["postcss-nested"]
      }
    ]
  ]
}

But I got an error since babel and Turbopack don't work together?

CleanShot 2024-03-07 at 00 14 14@2x

vpontis avatar Mar 07 '24 05:03 vpontis

You can't use turbopack for it

kdy1 avatar Mar 07 '24 06:03 kdy1

@kdy1 so why is this issue closed? It sounds like we are in the same position as before this issue, right?

Is this issue closed as wontfix or solved? If it's solved, how do I compile nested CSS in Styled JSX without using Babel?

Why did you close this issue? I'm not sure what new functionality in Next.js addresses the original issue. Is there new functionality? If so, how do I enable it?

I know you are busy (and you are doing great work with swc) but I'd appreciate if you could point me in the correct direction. Your past comments have made me more confused and I've spent a few hours trying to figure out how this works and what you are saying.

vpontis avatar Mar 07 '24 07:03 vpontis

Turbopack supports it. That's why this issue is closed. But we don't have next build --turbo. So if you want to build your app for production, you need to use webpack and babel. And turbopack does not support babel.

If your app is in development mode only, you can set the appropriate browserslist config to enable CSS nesting.

kdy1 avatar Mar 07 '24 07:03 kdy1

If your app is in development mode only, you can set the appropriate browserslist config to enable CSS nesting.

Can you share how to do this? I've tried a bunch of different things and have gone back and forth with the Vercel team but I haven't figured out how to get it working for development.

I would think that the default browserslist would compile nested CSS because older versions of Safari don't support CSS nesting.

I have made an example repo and tried like 10 different ways to get this enabled in development mode. Can you let me know how I can change my repo to get it working?

vpontis avatar Mar 07 '24 07:03 vpontis

@vpontis, what @kdy1 is trying to articulate, I think, is:

  • some degree of support was added in Turbopack for transpiling nested CSS selectors when the browserlist is specified
  • however, since we don't support turbopack for builds ATM, it won't fix production issues for you
  • so @kdy1 suggestion was that you should still use webpack for this (the default ATM)
  • you should use the babel config you linked to, just without running --turbo

feedthejim avatar Mar 07 '24 09:03 feedthejim

  • some degree of support was added in Turbopack for transpiling nested CSS selectors when the browserlist is specified

How do I enable this? I have tried multiple browserlists configs in package.json while running dev --turbo but it doesn't work. Has anyone been able to get this working on dev?

  • however, since we don't support turbopack for builds ATM, it won't fix production issues for you
  • so @kdy1 suggestion was that you should still use webpack for this (the default ATM)
  • you should use the babel config you linked to, just without running --turbo

Should this issue be re-opened? Or should I open another issue? The issue is "Turbopack does not compile CSS nesting for styled-jsx" — so if it only compiles it in a subset of cases (which I have not encountered) — then the issue should still be open.

vpontis avatar Mar 07 '24 15:03 vpontis

looked into it, I think one thing that @kdy1 might have overlooked is that the browserlists might not get consumed properly. We're investigating.

feedthejim avatar Mar 08 '24 10:03 feedthejim

Investigation

browserslist has multiple values, even in a single app.

[turbo/crates/turbopack-ecmascript-plugins/src/transform/styled_jsx.rs:35] &__self.target_browsers = BrowserData {
    chrome: Some(
        Version {
            major: 121,
            minor: 0,
            patch: 0,
        },
    ),
    chrome_android: None,
    firerfox_android: None,
    opera_android: None,
    quest: None,
    react_native: None,
    and_chr: None,
    and_ff: None,
    op_mob: None,
    ie: None,
    edge: Some(
        Version {
            major: 120,
            minor: 0,
            patch: 0,
        },
    ),
    firefox: Some(
        Version {
            major: 122,
            minor: 0,
            patch: 0,
        },
    ),
    safari: Some(
        Version {
            major: 17,
            minor: 3,
            patch: 0,
        },
    ),
    node: None,
    ios: None,
    samsung: None,
    opera: None,
    android: None,
    electron: None,
    phantom: None,
    opera_mobile: None,
    rhino: None,
    deno: None,
    hermes: None,
    oculus: None,
    bun: None,
}
[next.js/packages/next-swc/crates/next-core/src/next_shared/transforms/styled_jsx.rs:20] &versions = BrowserData {
    chrome: None,
    chrome_android: None,
    firerfox_android: None,
    opera_android: None,
    quest: None,
    react_native: None,
    and_chr: None,
    and_ff: None,
    op_mob: None,
    ie: None,
    edge: None,
    firefox: None,
    safari: None,
    node: None,
    ios: None,
    samsung: None,
    opera: None,
    android: None,
    electron: None,
    phantom: None,
    opera_mobile: None,
    rhino: None,
    deno: None,
    hermes: None,
    oculus: None,
    bun: None,
}

kdy1 avatar Mar 11 '24 03:03 kdy1

I think turbopack is not applying browserslist-based things to dev mode, because it's waste of time for 99.999% (Many developers use recent browsers while developing)

kdy1 avatar Mar 11 '24 03:03 kdy1

I think turbopack is not applying browserslist-based things to dev mode, because it's waste of time for 99.999% (Many developers use recent browsers while developing)

Thanks for looking into this. What's the current status of this issue? Should we re-open it? It seems like my original issue wasn't fixed.

Is there a plan to support Turbopack and compiling Styled JSX so that I can drop Babel?

I feel pretty lost here.

vpontis avatar Mar 13 '24 21:03 vpontis

Hey @vpontis I had a look at the initial question to dig into the confusion between what @kdy1 and @feedthejim said and what you were asking about.

@kdy1 added support for CSS nesting, the official specification: https://www.w3.org/TR/css-nesting-1/ which is handled in styled-jsx by default now for both Webpack and Turbopack, the Turbopack documentation is not super clear on this as it was written with the assumption that postcss-nested is the same as CSS nesting, but it's unfortunately not, postcss-nested has additional features which are not in the spec, the spec calls this out in the green section as part of this section: https://www.w3.org/TR/css-nesting-1/#syntax. We'll update the documentation to reflect that it's not postcss-nested but the CSS nesting spec instead.

Specifically the example you gave of &_ to concat .inn and er to create .inner is not supported in the official spec. The & keyword is supported in the spec and does allow you to chain selectors, just not concat selectors, also the syntax is slightly different instead of &_ it's just &.

I've also had a look into the browserlist config not applying, it is indeed the case that we currently don't pass along the browserlist config but that is not the intended outcome, we're going to add support for passing browserslist to Turbopack (including applying to the CSS) soon, @wbinnssmith is opening an issue for it on the turbo repository right now and will post back here when it's added to the issue tracker. Update: Issue was created here: https://github.com/vercel/next.js/issues/63430

Does that answer all your questions? If not let me know and I can provide additional details.

If you have more Babel config let me know as well, if it's only adding postcss-nested to styled-jsx it should be straightforward to migrate from postcss-nested to the official spec.

timneutkens avatar Mar 18 '24 20:03 timneutkens

Thank you Tim! So to clarify my understanding:

  1. This uses the CSS spec rather than postcss-nested spec
  2. The current version of Next.js is not compiling CSS in Styled JSX according to the CSS spec because Turbopack doesn't get the browserslist properly. There is an issue to fix that.
  3. Once the issue in 2 is fixed, Turbopack will compile CSS prefixes in development mode. But Turbopack will not be ready for production builds.
  4. There are plans to make Turbopack work for production builds.

If that is right, then once step 4 is complete, we should be able to drop Babel entirely.

vpontis avatar Mar 19 '24 03:03 vpontis

  1. Both the Turbopack and Webpack implementation of styled-jsx use the CSS nesting spec
  2. The current version of Next.js supports CSS nesting spec in styled-jsx, regardless of if you use turbopack or webpack. The Turbopack implementation doesn't support you passing lower targets using browserslist, but that doesn't mean you can't use CSS nesting.
  3. When we add browserslist support to Turbopack the CSS will have prefixing
  4. We're first focussing on development for Turbopack, once that is stable we'll shift our focus to production builds using Turbopack.

If that is right, then once step 4 is complete, we should be able to drop Babel entirely.

Removing Babel is unrelated to using Turbopack, you can already do this today to use SWC instead. Can you share your babel config so that I can confirm that?

timneutkens avatar Mar 19 '24 08:03 timneutkens

Additional clarification: This is the correct syntax for CSS nesting spec:

.container {
    color: blue;
    padding: 3rem;

    .inner {
        color: yellow
    }
}

Output:

.container {
    color: blue;
    padding: 3rem;
}

.container .inner {
    color: yellow;
}

The & keyword can be used for concatting:

.container {
    color: blue;
    padding: 3rem;

    &.inner {
         color: yellow
    }
}

Output:

.container {
    color: blue;
    padding: 3rem;
}

.container.inner {
    color: yellow;
}

You can try it in the LightningCSS playground

timneutkens avatar Mar 19 '24 08:03 timneutkens

Thanks Tim! I think there is one last point of confusion.

The current version of Next.js supports CSS nesting spec in styled-jsx, regardless of if you use turbopack or webpack. > Removing Babel is unrelated to using Turbopack, you can already do this today to use SWC instead.

You could mean two things by "current version of Next.js supports CSS nesting spec in styled-jsx":

  1. If you have nested CSS in Styled JSX, then Next.js won't crash. It will continue to output nested CSS. — I have found this to be true.
  2. If you have nested CSS in Styled JSX, then Next.js can compile that nested CSS with SWC to output CSS that has been unnested. — I haven't figured out how to get this working. If this is working, I can remove Babel entirely.

I recorded a 2 minute video showing how I am not able to get nested CSS to compile. You can see the demo repository I used in that video.

vpontis avatar Mar 19 '24 14:03 vpontis

Hey Victor, I did some further digging and you're right that styled-jsx's CSS processing does not downlevel currently, however that is a bug. We've discussed a fix and the initial work by kdy has been done for that here: https://github.com/swc-project/plugins/pull/275. For Turbopack, which uses https://lightningcss.dev/ what was missing is passing the browserslist target (i.e. the default browserslist target), which we're also adding. When both those changes land it will downlevel the nested selectors 👍 Will post back here when that is ready.

timneutkens avatar Mar 22 '24 12:03 timneutkens

Hey Victor,

I did some further digging and you're right that styled-jsx's CSS processing does not downlevel currently, however that is a bug. We've discussed a fix and the initial work by kdy has been done for that here: https://github.com/swc-project/plugins/pull/275. For Turbopack, which uses https://lightningcss.dev/ what was missing is passing the browserslist target (i.e. the default browserslist target), which we're also adding. When both those changes land it will downlevel the nested selectors 👍 Will post back here when that is ready.

Thanks Tim! I think I published all of the information required to report this bug a few times in multiple different formats above in this thread.

It's been a bit frustrating on my side. I truly appreciate the work y'all do but I felt like my multiple reproductions and examples have not been taken seriously until you took another look at things.

I hope you can fix this. It would be certainly very helpful for us. Even before I created this issue, this is something I've been looking into for over a year.

vpontis avatar Mar 22 '24 19:03 vpontis

Should be fixed by https://github.com/vercel/next.js/pull/63541

kdy1 avatar Mar 25 '24 04:03 kdy1