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

Native CSS nesting support for css modules #55053

Closed
1 task done
equinusocio opened this issue Sep 6, 2023 · 28 comments · Fixed by #62644
Closed
1 task done

Native CSS nesting support for css modules #55053

equinusocio opened this issue Sep 6, 2023 · 28 comments · Fixed by #62644
Labels
bug Issue was opened via the bug report template. locked

Comments

@equinusocio
Copy link

equinusocio commented Sep 6, 2023

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

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

@equinusocio equinusocio added the bug Issue was opened via the bug report template. label Sep 6, 2023
@samir9saeedi
Copy link

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

@equinusocio
Copy link
Author

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

samir9saeedi commented Sep 26, 2023

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

equinusocio commented Oct 2, 2023

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

equinusocio commented Nov 12, 2023

Is this issue planned? Any interest?

@davidyarham
Copy link

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

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

@CamiloGdev
Copy link

CamiloGdev commented Nov 24, 2023

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

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

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

@Leviathan91
Copy link

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

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

@equinusocio
Copy link
Author

equinusocio commented Jan 18, 2024

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

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

n2k3 commented Feb 5, 2024

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

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

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

@EdwardBock
Copy link

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

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

@acidfernando
Copy link

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

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

instead of relying on native CSS syntax

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

samcx added a commit that referenced this issue Feb 29, 2024
today ~83% of all browsers support css nesting:
https://caniuse.com/css-nesting

![CSS Nesting Browser
Support](https://github.com/css-modules/postcss-modules-local-by-default/assets/4113649/141f8dce-a8bd-4df4-b2bd-210252189711)
https://caniuse.com/css-nesting  
https://developer.mozilla.org/en-US/docs/Web/CSS/Nesting_selector

therefore this pr upgrades postcss-modules-local-by-default which fixes
a bug in
css-modules/postcss-modules-local-by-default#64)

-  `.foo { &:hover { a_value: some-value; } }` is pure
-  `.foo { html &:hover { a_value: some-value; } }` is pure
-  `.foo { &:global(.bar) { a_value: some-value; } }` is pure
-  `:global(.foo) { &:hover { a_value: some-value; } }` is **not** pure


upgrading the package will allow using css nestings with or **without**
postcss compilation

it fixes the following error:

```
CssSyntaxError: postcss-modules-local-by-default: <css input>:1:8: Selector "&:hover" is not pure (pure selectors must contain at least one local class or id)
```

Fixes #55053
Fixes #33734
Fixes #10475

Co-authored-by: Sam Ko <sam@vercel.com>
@vicentel89
Copy link

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

image
image
image

@samcx
Copy link
Member

samcx commented Mar 2, 2024

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

@equinusocio
Copy link
Author

equinusocio commented Mar 4, 2024

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 samcx reopened this Mar 4, 2024
@samcx
Copy link
Member

samcx commented Mar 4, 2024

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

@samcx samcx closed this as completed Mar 4, 2024
@equinusocio
Copy link
Author

equinusocio commented Mar 12, 2024

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

samcx commented Mar 12, 2024

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

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 Mar 27, 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. locked
Projects
None yet
Development

Successfully merging a pull request may close this issue.