vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.79k stars 26.95k forks source link

Native CSS nesting support for css modules #55053

Closed equinusocio closed 8 months ago

equinusocio commented 1 year ago

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/magical-chatelet-9ddsys?file=%2Fnext.config.js%3A4%2C3

To Reproduce

  1. Create any css module file
  2. Write a valid native CSS nested code

    .Layout {
      border: 10px solid red;
    
      & body {
        height: 100vh;
        margin: 0;
        border: 10px solid blue;
      }
    }
    
    /* or */
    
    .Layout {
      border: 10px solid red;
    
      body {
        height: 100vh;
        margin: 0;
        border: 10px solid blue;
      }
    }
  3. See the error about pure selectors

Current vs. Expected behavior

Nextjs and the check it performs on CSS modules prevents from using the new CSS native nesting syntax (which is now supported by evergreen browsers).

It returns error about a simple nested CSS while this should be allowed as valid CSS syntax now.

Error:

Selector "& body" is not pure (pure selectors must contain at least one local class or id)

See the reproduction link.

Next should allow native and valid CSS syntax and avoid extra checks on "pure selectors" where not needed.

Verify canary release

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP PREEMPT_DYNAMIC Sun Aug  6 20:05:33 UTC 2023
    Binaries:
      Node: 16.17.0
      npm: 8.15.0
      Yarn: 1.22.19
      pnpm: 7.1.0
    Relevant packages:
      next: 13.0.8-canary.0
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

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

Not sure

Additional context

The selector .Layout { & body {} } should be validated as .Layout body {} which is a "pure" css module selector.

samir9saeedi commented 1 year ago

FWIW you can use this PostCSS plugin in the meantime: https://github.com/csstools/postcss-plugins/tree/main/plugins/postcss-nesting

equinusocio commented 1 year ago

Thank you, we have used it since 2016 but now that we have the native nesting we need to remove it to reduce the css size.

samir9saeedi commented 1 year ago

No worries. The plugin I've linked to (postcss-nesting) supports the new official syntax, which is slightly different from what SASS uses. That syntax has its own plugin as well (postcss-nested), which is what you're most likely referring to. Doesn't make a difference in the bundle size presumably, just more future-proof.

equinusocio commented 1 year ago

No worries. The plugin I've linked to (postcss-nesting) supports the new official syntax, which is slightly different from what SASS uses. That syntax has its own plugin as well (postcss-nested), which is what you're most likely referring to. Doesn't make a difference in the bundle size presumably, just more future-proof.

Yep, we have used postcss-nesting since 2016 via postcss-preset-env. Now that the native nesting is available (and is pretty much the same as postcss-nesting) we would like to disable this plugin, we want nested CSS in the browser since the CSS size is reduced drastically.

Doesn't make a difference in the bundle size presumably, just more future-proof.

Yep, it affects the final CSS drastically, here is a basic example:

.MyLongClassName {
  color: red;

  &[data-button-color="blue"] {
    color: blue;
  }

  &[data-button-color="green"] {
    color: green;
  }  
}

All of this, when processed, becomes:

.MyLongClassName { color: red }
.MyLongClassName[data-button-color="blue"] { color: blue }
.MyLongClassName[data-button-color="green"] { color: green } 

As you can see the parent selector is repeated over and over, now multiply this for every nested selector. Without processing the nested syntax, we just get the exact same source code in browsers:

.MyLongClassName {
  color: red;
  &[data-button-color="blue"] { color: blue }
  &[data-button-color="blue"] { color: blue }
} 

139 chars minified vs. 108 chars minified. On our side, this is a vast improvement considering the amount of CSS we ship. 😌

warning

Personal opinion ahead

Also, and this is a personal opinion, there is no reason to prevent the usage of a native CSS feature. Doesn't seem fair.

equinusocio commented 11 months ago

Is this issue planned? Any interest?

davidyarham commented 11 months ago

I hope this is planned. Native nesting has been around since March 2023, and this week we have also got relaxed nesting rules natively.

I also agree with @equinusocio, I would rather not have processed CSS when its not needed and let the platform do the work when its supported.

landsman commented 11 months ago

I would love to see this. Currently builder blocking me from using them.

CamiloGdev commented 11 months ago

Link to the code that reproduces this issue or a replay of the bug

https://codesandbox.io/p/sandbox/magical-chatelet-9ddsys?file=%2Fnext.config.js%3A4%2C3

To Reproduce

  1. Create any css module file
  2. Write a valid native CSS nested code

    .Layout {
      border: 10px solid red;
    
      & body {
        height: 100vh;
        margin: 0;
        border: 10px solid blue;
      }
    }
    
    /* or */
    
    .Layout {
      border: 10px solid red;
    
      body {
        height: 100vh;
        margin: 0;
        border: 10px solid blue;
      }
    }
  3. See the error about pure selectors

Current vs. Expected behavior

Nextjs and the check it performs on CSS modules prevents from using the new CSS native nesting syntax (which is now supported by evergreen browsers).

It returns error about a simple nested CSS while this should be allowed as valid CSS syntax now.

Error:

Selector "& body" is not pure (pure selectors must contain at least one local class or id)

See the reproduction link.

Next should allow native and valid CSS syntax and avoid extra checks on "pure selectors" where not needed.

Verify canary release

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

Provide environment information

Operating System:
      Platform: linux
      Arch: x64
      Version: #1 SMP PREEMPT_DYNAMIC Sun Aug  6 20:05:33 UTC 2023
    Binaries:
      Node: 16.17.0
      npm: 8.15.0
      Yarn: 1.22.19
      pnpm: 7.1.0
    Relevant packages:
      next: 13.0.8-canary.0
      eslint-config-next: N/A
      react: 18.2.0
      react-dom: 18.2.0

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

Not sure

Additional context

The selector .Layout { & body {} } should be validated as .Layout body {} which is a "pure" css module selector.

hello my

I had the same problem as you and I'll show you how I solved it:

In this case, nesting only works if the selector to be nested is a class or an id, but if it is the selector of a label it does not work directly, but you can solve this if you name the parent class from which you are nesting again:

     .Layout {
       border: 10px solid red;

       &.layout body {
         height: 100vh;
         margin: 0;
         border: 10px solid blue;
       }
     } 

That is to say that as long as you start with a class or id the nesting and from then on you can call the selectors you want, it is somewhat strange but at least it solves it, but on the other hand I was very happy working this way and then I realized Note that it only works well in client components; I work with Next and it turns out that with client components, nesting CSS in this way works, but as soon as I use it in a server component it stops working, it correctly identifies the classes and their nesting, but it omits properties like flex or some others , this is even rarer, for example

.dataBox {
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
    gap: 1rem;
    color: var(--color-S01-800);
    width: 100%;
    border: 1px solid;
    border-color: var(--color-T1-400);
    border-radius: 2rem;
    padding: 4rem;

    .dataTitleWrapper {
        display: flex;
        justify-content: space-between;
        align-items: center;
        gap: 1rem;
        width: 100%;
    }

    .inlineField {
        width: 100%;
        display: flex;
        gap: 1rem;
        justify-content: center;
        align-items: center;
    }
}

In this case, all the properties related to flex in both the .inlineField and .dataTitleWrapper classes, for some reason, when checking with devtools in the browser, they do not appear as if I had not written them and therefore the styles come out wrong, but if I pass them without the nesting, everything it turns out well. but if I pass that same code to a client component everything works out fine.

I DON'T UNDERSTAND ANYTHING

Sorry for my English, I only speak Spanish, it's a pure translator

teddarcuri commented 11 months ago

I'd also like to see this feature added/allowed.

Leviathan91 commented 9 months ago

Since this feature is not included in Nextjs yet, an intermediate solution has been propsed: using postcss-nesting plugin. I am using NextJs14 with the app directory. While there are docs on how to integerate postcss into pages router there seem to be no docs for postcss integration for the app router I just tried on a new project with the default settings (npx@latest, spam click enter) and then adding postcss-nesting and a postcss config file does not work. What am I missing here? I'd really like to start using the nested css syntax and then lateron remove postcss when the feature is included in canary, instead of having to re-write the css then.

landsman commented 9 months ago

I would love to make this work natively with CSS modules.

equinusocio commented 9 months ago

Since this feature is not included in Nextjs yet, an intermediate solution has been propsed: using postcss-nesting plugin.

I am using NextJs14 with the app directory. While there are docs on how to integerate postcss into pages router

there seem to be no docs for postcss integration for the app router

I just tried on a new project with the default settings (npx@latest, spam click enter) and then adding postcss-nesting and a postcss config file does not work.

What am I missing here? I'd really like to start using the nested css syntax and then lateron remove postcss when the feature is included in canary, instead of having to re-write the css then.

Postcss-nesting is not a solution for the issue. It compiles the css by removing the nesting and duplicating the selectors. Plus by using browsers list if the target browsers support nesting, the css will stay the same (with nested css) and gives the same error.

melodyarcjason commented 9 months ago

Would like to add to this issue - it is making a lot of UI package updates VERY difficult. In my case - migrating @mantine v6 -> 7 to get away from the CSS-in-JS emotion style so that we are not required to 'use client' on everything.

Not only am I having to convert to the css module - but finding that my nesting is not working, so I have to refactor class names on the component code and flatten my css making the task quite a bit more painful. An example that I have using css module:

This doesn't apply css to the 'badge':

.taskHeader {
  padding: 0rem 1rem;
  display: flex;
  align-items: center;
  gap: 0.5rem;

  opacity: 0.75;
  font-size: 0.75rem;
  font-style: normal;
  font-weight: 600;
  line-height: 1.25rem;

  .mantine-Badge-root{
    height: 1rem;
    min-width: 1.5rem;
    padding: 0rem 0.1rem;
  }
}

But the badge is a 'div' so this works oddly enough:

.taskHeader {
  padding: 0rem 1rem;
  display: flex;
  align-items: center;
  gap: 0.5rem;

  opacity: 0.75;
  font-size: 0.75rem;
  font-style: normal;
  font-weight: 600;
  line-height: 1.25rem;

  & > div {
    height: 1rem;
    min-width: 1.5rem;
    padding: 0rem 0.1rem;
  }
}

As a test I tried using & > .mantine-Badge-root and that does not work as a nested selector either. I even tried & > div.mantine-Badge-root and that too fails.

Would be great to get basic css class nesting in place to help adoption and migration efforts.

n2k3 commented 9 months ago

I'm in the same boat, build error when using CSS modules nested selector.

image

It seems that when adding --turbo to the dev server, like so in package.json:

"scripts": {
    "dev": "next dev --turbo"
}

This results in same behaviour as the postcss-nesting package mentioned above, it at least works for local development.

/* [project]/src/components/AppHeader.module.css [app-client] (css) */
.topButtons__AppHeader__9b530d28 {
  position: absolute;
  right: 0;
}
.topButtons__AppHeader__9b530d28 button:last-of-type {
  padding-right: 0;
}

This won't work for shipping production builds, as Turbopack doesn't support that yet.

Would be nice to see this supported natively in Next.js 👍

SSTPIERRE2 commented 9 months ago

We can't just forget about core CSS features like this while supporting the bleeding edge of React. The community at large has been anticipating built-in CSS nesting for a long time, and now Next.js devs are still waiting.

equinusocio commented 8 months ago

After 5 months no response from them. I guess they don't care.

EdwardBock commented 8 months ago

That is very sad because it is a blocker in my agency. If we cannot trust the system to support at least the browsers core features, we cannot use it.

legowerewolf commented 8 months ago

Well, damn. I was hoping to switch away from SCSS because all I was using it for was the nesting syntax.

acidfernando commented 8 months ago

I'm not sure what nextjs uses to parse the CSS modules, but css-loader already has fixed this same issue 2 months ago.

Supposing they use css-loader, they just have to update.

equinusocio commented 8 months ago

I'm not sure what nextjs uses to parse the CSS modules, but css-loader already has fixed this same issue 2 months ago.

Supposing they use css-loader, they just have to update.

Doesn't seems a fix because CSS nesting doesn't require & at the beginning of the selector. The fix they pushed seems to force people to use & instead of relying on native CSS syntax. This is a boo boo.

ryami333 commented 8 months ago

instead of relying on native CSS syntax

Well, the & is also native CSS syntax, so it is a partial fix, at least.

vicentel89 commented 8 months ago

I am using version 14.1.1 and I still get the error.

image image image

samcx commented 8 months ago

@vicentel89 Can you try the latest :nextjs: canary instead?

equinusocio commented 8 months ago

Can you try the latest :nextjs: canary instead?

@samcx It seems to work only with the & as the starting symbol, which is not required by the native syntax. This issue is not solved since we should be able to use CSS nesting as is it by the spec, with or without &.

samcx commented 8 months ago

@equinusocio I think this is fine—my current understanding is that CSS Modules doesn't support CSS nesting without explicitly using &https://github.com/css-modules/postcss-modules-local-by-default/pull/64.

equinusocio commented 7 months ago

I am using version 14.1.1 and I still get the error.

image image image

Same here.

What happened to this fix? it seems fixed only in v14.1.1-canary.82 (the only one working). All other versions after this canary don't include the fix anywhere in the changelog or release and are not working.

Since 14.1.1 (which should include the fix from v14.1.1-canary.82), 14.1.2, and the latest 14.1.3 are not working this issue can't be considered fixed.

It's also blocking everyone targeting last 2 browsers versions in browserslist, forcing people to use a canary release in production.

samcx commented 7 months ago

@equinusocio The past two stable patch versions likely does not have this change (backported stable patch versions). The latest canary does indeed have the dependency bump.

github-actions[bot] commented 7 months ago

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.