vercel / next.js

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

Mismatch of names in grid-template-areas and grid-area across different css modules with --turbo #64509

Closed Arctomachine closed 5 months ago

Arctomachine commented 6 months ago

Link to the code that reproduces this issue

https://github.com/Arctomachine/next-grid-template-areas-bug

To Reproduce

  1. Start dev server with --turbo flag
  2. Observe devtools for different names of grid areas in grid and in cards, incorrect visuals

Current vs. Expected behavior

Expected: areas have same name in grid and cards Current: area names mismatch. Grid will append its module name to grid-template-area, but card will append its module name to grid-area. Since they are different names, browser will not put cards into grid correctly.

Example of area name in grid: image And in card: image

Provide environment information

Operating System:
  Platform: win32
  Arch: x64
  Version: Windows 10 Pro
  Available memory (MB): 32720
  Available CPU cores: 12
Binaries:
  Node: 20.10.0
  npm: N/A
  Yarn: N/A
  pnpm: N/A
Relevant Packages:
  next: 14.2.1-canary.6 // Latest available version is detected (14.2.1-canary.6).
  eslint-config-next: N/A
  react: 18.2.0
  react-dom: 18.2.0
  typescript: 5.1.3
Next.js Config:
  output: N/A

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

Turbopack (--turbo)

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

next dev (local)

Additional context

Only happens for turbopack in dev mode. Production build is not affected

PACK-2976

TariqSaiyad commented 6 months ago

I'm experiencing the same issue. I've got a grid container in the header.module.scss file: header-module-scss-module__I8X9PG__Level1

and the same grid-area name in the mega-menu.module.scss file: mega--menu-module-scss-module__cuXXBW__Level1

This completely breaks the grid and layout

cpotey commented 6 months ago

Also experiencing this, and it's a pretty big blocker.

I work in a turborepo monorepo. Decided to try turbopack on a new Next.js app I was working on, and one of my shared components uses grid-template-areas, and has become completely confused and broken. Unsure whether to attempt to refactor things, or hope for a fix.

Think I'll just exclude turbopack/--turbo for now.

erikbrgn commented 6 months ago

I'm not entirely sure this is solely due to an issue in turbopack. It might as well be due to lightningcss doing something weird under-the-hood. Fortunately, there's a work-around for those of you who want to keep using CSS modules with grid-template-areas and grid-area until this is fixed properly.

Using turbopack in a nextjs project, this "setup" has worked for me:

/* layout.module.css */

.layout {
  grid-template-areas:
    'header header header header'
    'nav main main main'
    'nav main main main'
    'footer footer footer footer;
}

.header {
  grid-area: header;
}

.nav {
  grid-area: nav;
}

.main {
  grid-area: main;
}
/* header.module.css */

.header {
  composes: header from './layout.module.css';
  */ ... */
}
/* nav.module.css */

.nav {
  composes: nav from './layout.module.css';
  */ ... */
}
/* main.module.css */

.main {
  composes: main from './layout.module.css';
  */ ... */
}

And so on...

timneutkens commented 6 months ago

The reason these get prefixed is that we're using LightningCSS for processing CSS which is smarter with CSS Modules, it also ensures grid names don't conflict as per the documentation: https://lightningcss.dev/css-modules.html#css-grid

Webpack's CSS modules handling doesn't do this (but it probably should). That's what is causing the mismatch as you're upgrading.

My recommendation would be to restructure the grid templates to be defined in global CSS instead of CSS Modules.

Arctomachine commented 6 months ago

Global styles are good for general layout of whole site which would be used for every page. But they are not good for making small isolated component displayed in one place of entire site. This is what modules are intended for.

timneutkens commented 6 months ago

CSS Modules CSS rules are intended to be scoped, this includes e.g. CSS animations by default. The only reason grid wasn't prefixed before is that it's a newer CSS standard and webpack's CSS handling didn't have an implementation for it, whereas the more modern LightningCSS compiler we use in Turbopack does support it.

In the case that you're describing it's basically leaking grid names making them "global" even though they're defined in a CSS Module. Basically previously that was a bug with the webpack handling.

GabenGar commented 6 months ago

Your second explanation makes a lot more sense than the link in the lightningcss (which sounded like mumbo jumbo). This explanation should be added to docs somewhere and add an error page just for that (if it's possible to detect from code analysis). It's going to trip up a lot of people if it lands into webpack. It never occurred to me that grid name declarations are basically global declarations, in part because I've never used them outside of parent/children relations within a CSS module, but it does makes sense why a bundler must also generate unique grid area names out of them.

cpotey commented 5 months ago

Looks like this will be sorted in an upcoming release?

@timneutkens thanks for sorting this, appreciated ❤️

timneutkens commented 5 months ago

Yes indeed! I ended up talking to Devon (maintainer of Lightning CSS about it) and we agreed that having a way towards compatibility with webpack's CSS modules handling would be beneficial for people migrating to Lightning CSS and for Turbopack as everyone using it is coming from webpack's CSS modules handling right now 👍

More info here: https://x.com/timneutkens/status/1791586981337604443

Overall I still believe scoping grid is the right thing to do but having compatibility is more important 🙏

github-actions[bot] commented 5 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.