Skip to content

Commit 1d08dab

Browse files
sokrahuozhi
authored andcommitted
avoid merging global css in a way that leaks into other chunk groups (#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
1 parent 21a9d59 commit 1d08dab

File tree

8 files changed

+94
-0
lines changed

8 files changed

+94
-0
lines changed

packages/next/src/build/webpack/plugins/css-chunking-plugin.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ const MIN_CSS_CHUNK_SIZE = 30 * 1024
1111
*/
1212
const MAX_CSS_CHUNK_SIZE = 100 * 1024
1313

14+
function isGlobalCss(module: Module) {
15+
return !/\.module\.(css|scss|sass)$/.test(module.nameForCondition() || '')
16+
}
17+
1418
type ChunkState = {
1519
chunk: Chunk
1620
modules: Module[]
@@ -125,6 +129,8 @@ export class CssChunkingPlugin {
125129

126130
// Process through all modules
127131
for (const startModule of remainingModules) {
132+
let globalCssMode = isGlobalCss(startModule)
133+
128134
// The current position of processing in all selected chunks
129135
let allChunkStates = new Map(chunkStatesByModule.get(startModule)!)
130136

@@ -225,8 +231,36 @@ export class CssChunkingPlugin {
225231
}
226232
}
227233
}
234+
235+
// Global CSS must not leak into unrelated chunks
236+
const nextIsGlobalCss = isGlobalCss(nextModule)
237+
if (nextIsGlobalCss && globalCssMode) {
238+
if (allChunkStates.size !== nextChunkStates.size) {
239+
// Fast check
240+
continue
241+
}
242+
}
243+
if (globalCssMode) {
244+
for (const chunkState of nextChunkStates.keys()) {
245+
if (!allChunkStates.has(chunkState)) {
246+
// Global CSS would leak into chunkState
247+
continue loop
248+
}
249+
}
250+
}
251+
if (nextIsGlobalCss) {
252+
for (const chunkState of allChunkStates.keys()) {
253+
if (!nextChunkStates.has(chunkState)) {
254+
// Global CSS would leak into chunkState
255+
continue loop
256+
}
257+
}
258+
}
228259
potentialNextModules.delete(nextModule)
229260
currentSize += size
261+
if (nextIsGlobalCss) {
262+
globalCssMode = true
263+
}
230264
for (const [chunkState, i] of nextChunkStates) {
231265
if (allChunkStates.has(chunkState)) {
232266
// This reduces the request count of the chunk group
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#hello1,
2+
#hello2 {
3+
color: rgb(255, 0, 0);
4+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import '../base.css'
2+
import './style.css'
3+
import Nav from '../nav'
4+
5+
export default function Page() {
6+
return (
7+
<div>
8+
<p id="hello1">hello world</p>
9+
<Nav />
10+
</div>
11+
)
12+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#hello1,
2+
#hello2 {
3+
color: rgb(0, 255, 0);
4+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import '../base.css'
2+
import './style.css'
3+
import Nav from '../nav'
4+
5+
export default function Page() {
6+
return (
7+
<div>
8+
<p id="hello2">hello world</p>
9+
<Nav />
10+
</div>
11+
)
12+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#hello1,
2+
#hello2 {
3+
color: rgb(0, 0, 255);
4+
}

test/e2e/app-dir/css-order/app/nav.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,16 @@ export default function Nav() {
7070
Partial Reversed B
7171
</Link>
7272
</li>
73+
<li>
74+
<Link href={'/global-first'} id="global-first">
75+
Global First
76+
</Link>
77+
</li>
78+
<li>
79+
<Link href={'/global-second'} id="global-second">
80+
Global Second
81+
</Link>
82+
</li>
7383
</ul>
7484
<h3>Pages</h3>
7585
<ul>

test/e2e/app-dir/css-order/css-order.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,20 @@ const PAGES: Record<
183183
color: 'rgb(255, 55, 255)',
184184
background: 'rgba(0, 0, 0, 0)',
185185
},
186+
'global-first': {
187+
group: 'global',
188+
conflict: true,
189+
url: '/global-first',
190+
selector: '#hello1',
191+
color: 'rgb(0, 255, 0)',
192+
},
193+
'global-second': {
194+
group: 'global',
195+
conflict: true,
196+
url: '/global-second',
197+
selector: '#hello2',
198+
color: 'rgb(0, 0, 255)',
199+
},
186200
}
187201

188202
const allPairs = getPairs(Object.keys(PAGES))

0 commit comments

Comments
 (0)