Skip to content

Commit

Permalink
avoid merging global css in a way that leaks into other chunk groups (#…
Browse files Browse the repository at this point in the history
…67373)

### What?

This disallows merging of global css with styles that appear on other
pages/chunk groups.

### Why?

Before we made the assumption that all CSS is written in a way that it
only affects the elements it should really affect.

In general writing CSS in that way is recommended. In App Router styles
are only added and never removed. This means when a user uses
client-side navigations to navigate the application, styles from all
previous pages are still active on the current page. To avoid visual
artefacts one need to write CSS in a way that it only affects certain
elements. Usually this can be archived by using class names. CSS Modules
even enforce this recommendation.

Assuming that all styles are written this way allows to optimize CSS
loading as request count can be reduced when (small) styles are merged
together.

But turns out that some applications are written differently. They use
global styles that are not scoped to a class name (e. g. to `body`
directly instead) and use them in different sections of the application.
They are structured in a way that doesn't allow client-side navigations
between these sections. This should be valid too, which makes our
assumption not always holding true.

This PR changes the algorithm so we only make that assumption for CSS
Modules, but not for global CSS. While this affects the ability to
optimize, applications usually do not use too much global CSS files, so
that can be accepted.

fixes #64773
  • Loading branch information
sokra authored and huozhi committed Jul 9, 2024
1 parent 21a9d59 commit 1d08dab
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 0 deletions.
34 changes: 34 additions & 0 deletions packages/next/src/build/webpack/plugins/css-chunking-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const MIN_CSS_CHUNK_SIZE = 30 * 1024
*/
const MAX_CSS_CHUNK_SIZE = 100 * 1024

function isGlobalCss(module: Module) {
return !/\.module\.(css|scss|sass)$/.test(module.nameForCondition() || '')
}

type ChunkState = {
chunk: Chunk
modules: Module[]
Expand Down Expand Up @@ -125,6 +129,8 @@ export class CssChunkingPlugin {

// Process through all modules
for (const startModule of remainingModules) {
let globalCssMode = isGlobalCss(startModule)

// The current position of processing in all selected chunks
let allChunkStates = new Map(chunkStatesByModule.get(startModule)!)

Expand Down Expand Up @@ -225,8 +231,36 @@ export class CssChunkingPlugin {
}
}
}

// Global CSS must not leak into unrelated chunks
const nextIsGlobalCss = isGlobalCss(nextModule)
if (nextIsGlobalCss && globalCssMode) {
if (allChunkStates.size !== nextChunkStates.size) {
// Fast check
continue
}
}
if (globalCssMode) {
for (const chunkState of nextChunkStates.keys()) {
if (!allChunkStates.has(chunkState)) {
// Global CSS would leak into chunkState
continue loop
}
}
}
if (nextIsGlobalCss) {
for (const chunkState of allChunkStates.keys()) {
if (!nextChunkStates.has(chunkState)) {
// Global CSS would leak into chunkState
continue loop
}
}
}
potentialNextModules.delete(nextModule)
currentSize += size
if (nextIsGlobalCss) {
globalCssMode = true
}
for (const [chunkState, i] of nextChunkStates) {
if (allChunkStates.has(chunkState)) {
// This reduces the request count of the chunk group
Expand Down
4 changes: 4 additions & 0 deletions test/e2e/app-dir/css-order/app/base.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#hello1,
#hello2 {
color: rgb(255, 0, 0);
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/css-order/app/global-first/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import '../base.css'
import './style.css'
import Nav from '../nav'

export default function Page() {
return (
<div>
<p id="hello1">hello world</p>
<Nav />
</div>
)
}
4 changes: 4 additions & 0 deletions test/e2e/app-dir/css-order/app/global-first/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#hello1,
#hello2 {
color: rgb(0, 255, 0);
}
12 changes: 12 additions & 0 deletions test/e2e/app-dir/css-order/app/global-second/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import '../base.css'
import './style.css'
import Nav from '../nav'

export default function Page() {
return (
<div>
<p id="hello2">hello world</p>
<Nav />
</div>
)
}
4 changes: 4 additions & 0 deletions test/e2e/app-dir/css-order/app/global-second/style.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#hello1,
#hello2 {
color: rgb(0, 0, 255);
}
10 changes: 10 additions & 0 deletions test/e2e/app-dir/css-order/app/nav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,16 @@ export default function Nav() {
Partial Reversed B
</Link>
</li>
<li>
<Link href={'/global-first'} id="global-first">
Global First
</Link>
</li>
<li>
<Link href={'/global-second'} id="global-second">
Global Second
</Link>
</li>
</ul>
<h3>Pages</h3>
<ul>
Expand Down
14 changes: 14 additions & 0 deletions test/e2e/app-dir/css-order/css-order.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,20 @@ const PAGES: Record<
color: 'rgb(255, 55, 255)',
background: 'rgba(0, 0, 0, 0)',
},
'global-first': {
group: 'global',
conflict: true,
url: '/global-first',
selector: '#hello1',
color: 'rgb(0, 255, 0)',
},
'global-second': {
group: 'global',
conflict: true,
url: '/global-second',
selector: '#hello2',
color: 'rgb(0, 0, 255)',
},
}

const allPairs = getPairs(Object.keys(PAGES))
Expand Down

0 comments on commit 1d08dab

Please sign in to comment.