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

Turbopack does not compile CSS nesting for styled-jsx #57718

Closed
1 task done
vpontis opened this issue Oct 29, 2023 · 35 comments · Fixed by #60876 or #61433
Closed
1 task done

Turbopack does not compile CSS nesting for styled-jsx #57718

vpontis opened this issue Oct 29, 2023 · 35 comments · Fixed by #60876 or #61433
Assignees
Labels
bug Issue was opened via the bug report template. linear: turbopack Confirmed issue that is tracked by the Turbopack team. locked SWC Related to minification/transpilation in Next.js. Turbopack Related to Turbopack with Next.js.

Comments

@vpontis
Copy link

vpontis commented Oct 29, 2023

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

  • 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 vpontis added the bug Issue was opened via the bug report template. label Oct 29, 2023
@github-actions github-actions bot added SWC Related to minification/transpilation in Next.js. Turbopack Related to Turbopack with Next.js. labels Oct 29, 2023
@kdy1 kdy1 assigned kdy1 and unassigned kdy1 Oct 30, 2023
@kdy1
Copy link
Member

kdy1 commented Oct 30, 2023

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

@vpontis
Copy link
Author

vpontis commented Oct 30, 2023

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!

@ForsakenHarmony ForsakenHarmony added the linear: turbopack Confirmed issue that is tracked by the Turbopack team. label Dec 6, 2023
@vpontis
Copy link
Author

vpontis commented Dec 19, 2023

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. 🙏

@leerob
Copy link
Member

leerob commented Jan 10, 2024

@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.

@vpontis
Copy link
Author

vpontis commented Jan 10, 2024

@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...

@kdy1 kdy1 self-assigned this Jan 18, 2024
kdy1 added a commit that referenced this issue Jan 24, 2024
# Turbopack

* vercel/turborepo#7027 <!-- Donny/강동윤 - Update `swc_core` to `v0.87.28` -->

---

### What?

Update swc crates

### Why?

Required for #57718.
`styled-jsx` crate now has a hook to transform CSS code using a
Rust-side API

### How?

Fixes #57718



Closes PACK-2256
@vpontis
Copy link
Author

vpontis commented Jan 24, 2024

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

@kdy1
Copy link
Member

kdy1 commented Jan 30, 2024

Opening as this is reverted.
vercel/turborepo#7163

@kdy1 kdy1 reopened this Jan 30, 2024
kdy1 added a commit to vercel/turborepo that referenced this issue Jan 30, 2024
### Description

This PR partially reverts #7027 to fix build issues.

 - Reopens vercel/next.js#57718

### Testing Instructions

Local override will work

Closes PACK-2322
kdy1 added a commit that referenced this issue Jan 31, 2024
# Turbopack

* vercel/turborepo#7167 <!-- Leah - fix: catch
import map lookup error properly in `ResolvingIssue` -->
* vercel/turborepo#7182 <!-- Donny/강동윤 - feat:
Re-enable `preset-env` mode of `styled-jsx` in turbopack -->
* vercel/turborepo#7161 <!-- Tobias Koppers - add
support for globs in sideEffects field in package.json -->
* vercel/turborepo#7184 <!-- Donny/강동윤 - chore:
Update `swc_core` to `v0.89.6` -->

### What?

Update turbopack

### Why?

To test vercel/turborepo#7182 against internal
webapps.

### How?

 - Closes #57718

Closes PACK-2334
Copy link
Contributor

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 github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2024
@vercel vercel unlocked this conversation Mar 6, 2024
@kdy1
Copy link
Member

kdy1 commented Mar 6, 2024

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

@vpontis
Copy link
Author

vpontis commented Mar 6, 2024

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

@kdy1
Copy link
Member

kdy1 commented Mar 7, 2024

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

@vpontis
Copy link
Author

vpontis commented Mar 7, 2024

@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

@kdy1
Copy link
Member

kdy1 commented Mar 7, 2024

You can't use turbopack for it

@vpontis
Copy link
Author

vpontis commented Mar 7, 2024

@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.

@kdy1
Copy link
Member

kdy1 commented Mar 7, 2024

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.

@vpontis
Copy link
Author

vpontis commented Mar 7, 2024

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?

@feedthejim
Copy link
Contributor

@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

@vpontis
Copy link
Author

vpontis commented Mar 7, 2024

  • 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.

@feedthejim
Copy link
Contributor

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

@kdy1
Copy link
Member

kdy1 commented Mar 11, 2024

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
Copy link
Member

kdy1 commented Mar 11, 2024

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)

@vpontis
Copy link
Author

vpontis commented Mar 13, 2024

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.

@timneutkens
Copy link
Member

timneutkens commented Mar 18, 2024

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: #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.

@vpontis
Copy link
Author

vpontis commented Mar 19, 2024

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.

@timneutkens
Copy link
Member

timneutkens commented Mar 19, 2024

  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
Copy link
Member

timneutkens commented Mar 19, 2024

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

@vpontis
Copy link
Author

vpontis commented Mar 19, 2024

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.

@timneutkens
Copy link
Member

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: swc-project/plugins#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 timneutkens reopened this Mar 22, 2024
@vpontis
Copy link
Author

vpontis commented Mar 22, 2024

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: swc-project/plugins#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.

@kdy1
Copy link
Member

kdy1 commented Mar 25, 2024

Should be fixed by #63541

@kdy1 kdy1 closed this as completed Mar 25, 2024
@vpontis
Copy link
Author

vpontis commented Mar 25, 2024

@kdy1 — we've gone back and forth many times on this issue. You've marked it "fixed" a few times but I never know what that is supposed to mean.

Can you share what you mean by "fixed"? How do I reproduce your fix? Is it fixed for Turbopack? For SWC? Can you please, please use a few sentences to describe the fix and how you verified it. I will also spend time tomorrow trying to figure out and verify your fix.

@kdy1
Copy link
Member

kdy1 commented Mar 25, 2024

  1. I made styled-jsx plugin force-apply compatibility CSS transforms in any mode. feat(styled-jsx): Apply CSS compatibility transforms swc-project/plugins#275
  2. I updated styled-jsx of turbopack. Update swc_core to v0.90.24 turborepo#7796
  3. Update turbopack #63541 updated turbopack of next.js

@vpontis
Copy link
Author

vpontis commented Mar 25, 2024

Thank you for the explanation! I think I'll need to wait until the next Canary release is published before testing this, is that right?

Will this mean that:

  1. SWC alone will compile nested CSS classes in Styled JSX so that if I run pnpm dev (without Turbo), the nested CSS will be compiled? I am guessing this will also apply to production builds.
  2. Turbopack will also compile nested CSS classes. I know that Turbopack is not applicable for production builds yet.

@vpontis
Copy link
Author

vpontis commented Mar 26, 2024

I got it working on dev (without Turbopack)!!!!!! Incredible!!!!!!!!!!!!!!!!!!!!

Copy link
Contributor

github-actions bot commented Apr 9, 2024

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 github-actions bot locked as resolved and limited conversation to collaborators Apr 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template. linear: turbopack Confirmed issue that is tracked by the Turbopack team. locked SWC Related to minification/transpilation in Next.js. Turbopack Related to Turbopack with Next.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants