Skip to content

Commit

Permalink
Fix unused CSS module imports are tracked on the server (vercel#40996)
Browse files Browse the repository at this point in the history
Reported by @hanneslund, when a CSS modules file gets imported in server
components, during `collectClientComponentsAndCSSForDependency` in our
client entry plugin it will always be collected no matter it is used or
not. Due to the restriction that we have to collect these imports to
create the client entry, it has to run in the `finishMake` compiler
phase and at that time, module optimization hasn't started yet.

To fix that issue, we run another pass in `afterOptimizeModules` just to
collect CSS imports for the server style manifest and we can filter out
unused modules there.

## Bug

- [ ] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
2 people authored and BowlingX committed Oct 5, 2022
1 parent 10a7a0a commit 76a8d49
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 28 deletions.
107 changes: 79 additions & 28 deletions packages/next/build/webpack/plugins/flight-client-entry-plugin.ts
Expand Up @@ -14,6 +14,7 @@ import type {
import { APP_DIR_ALIAS } from '../../../lib/constants'
import {
COMPILER_NAMES,
EDGE_RUNTIME_WEBPACK,
FLIGHT_SERVER_CSS_MANIFEST,
} from '../../../shared/lib/constants'
import { FlightCSSManifest, traverseModules } from './flight-manifest-plugin'
Expand Down Expand Up @@ -84,35 +85,51 @@ export class FlightClientEntryPlugin {
ReturnType<typeof this.injectClientEntryAndSSRModules>
> = []

// For each SC server compilation entry, we need to create its corresponding
// client component entry.
for (const [name, entry] of compilation.entries.entries()) {
// Check if the page entry is a server component or not.
const entryDependency: webpack.NormalModule | undefined =
entry.dependencies?.[0]
// Ensure only next-app-loader entries are handled.
if (!entryDependency || !entryDependency.request) continue

const request = entryDependency.request
// Loop over all the entry modules.
function forEachEntryModule(
callback: ({
name,
entryModule,
}: {
name: string
entryModule: any
}) => void
) {
for (const [name, entry] of compilation.entries.entries()) {
// Check if the page entry is a server component or not.
const entryDependency: webpack.NormalModule | undefined =
entry.dependencies?.[0]
// Ensure only next-app-loader entries are handled.
if (!entryDependency || !entryDependency.request) continue

const request = entryDependency.request

if (
!request.startsWith('next-edge-ssr-loader?') &&
!request.startsWith('next-app-loader?')
)
continue

if (
!request.startsWith('next-edge-ssr-loader?') &&
!request.startsWith('next-app-loader?')
)
continue
let entryModule: webpack.NormalModule =
compilation.moduleGraph.getResolvedModule(entryDependency)

let entryModule: webpack.NormalModule =
compilation.moduleGraph.getResolvedModule(entryDependency)
if (request.startsWith('next-edge-ssr-loader?')) {
entryModule.dependencies.forEach((dependency) => {
const modRequest: string | undefined = (dependency as any).request
if (modRequest?.includes('next-app-loader')) {
entryModule =
compilation.moduleGraph.getResolvedModule(dependency)
}
})
}

if (request.startsWith('next-edge-ssr-loader?')) {
entryModule.dependencies.forEach((dependency) => {
const modRequest: string | undefined = (dependency as any).request
if (modRequest?.includes('next-app-loader')) {
entryModule = compilation.moduleGraph.getResolvedModule(dependency)
}
})
callback({ name, entryModule })
}
}

// For each SC server compilation entry, we need to create its corresponding
// client component entry.
forEachEntryModule(({ name, entryModule }) => {
const internalClientComponentEntryImports = new Set<
ClientComponentImports[0]
>()
Expand All @@ -123,15 +140,13 @@ export class FlightClientEntryPlugin {
const layoutOrPageDependency = connection.dependency
const layoutOrPageRequest = connection.dependency.request

const [clientComponentImports, cssImports] =
const [clientComponentImports] =
this.collectClientComponentsAndCSSForDependency({
layoutOrPageRequest,
compilation,
dependency: layoutOrPageDependency,
})

Object.assign(flightCSSManifest, cssImports)

const isAbsoluteRequest = layoutOrPageRequest[0] === '/'

// Next.js internals are put into a separate entry.
Expand Down Expand Up @@ -170,7 +185,28 @@ export class FlightClientEntryPlugin {
bundlePath: 'app-internals',
})
)
}
})

// After optimizing all the modules, we collect the CSS that are still used.
compilation.hooks.afterOptimizeModules.tap(PLUGIN_NAME, () => {
forEachEntryModule(({ entryModule }) => {
for (const connection of compilation.moduleGraph.getOutgoingConnections(
entryModule
)) {
const layoutOrPageDependency = connection.dependency
const layoutOrPageRequest = connection.dependency.request

const [, cssImports] =
this.collectClientComponentsAndCSSForDependency({
layoutOrPageRequest,
compilation,
dependency: layoutOrPageDependency,
})

Object.assign(flightCSSManifest, cssImports)
}
})
})

compilation.hooks.processAssets.tap(
{
Expand Down Expand Up @@ -261,6 +297,21 @@ export class FlightClientEntryPlugin {
const isClientComponent = isClientComponentModule(mod)

if (isCSS) {
const sideEffectFree =
mod.factoryMeta && (mod.factoryMeta as any).sideEffectFree

if (sideEffectFree) {
const unused = !compilation.moduleGraph
.getExportsInfo(mod)
.isModuleUsed(
this.isEdgeServer ? EDGE_RUNTIME_WEBPACK : 'webpack-runtime'
)

if (unused) {
return
}
}

serverCSSImports[layoutOrPageRequest] =
serverCSSImports[layoutOrPageRequest] || []
serverCSSImports[layoutOrPageRequest].push(modRequest)
Expand Down
1 change: 1 addition & 0 deletions test/e2e/app-dir/app/app/css/css-page/page.js
@@ -1,4 +1,5 @@
import './style.css'

import styles from './style.module.css'

export default function Page() {
Expand Down
12 changes: 12 additions & 0 deletions test/e2e/app-dir/app/app/css/css-page/unused/page.js
@@ -0,0 +1,12 @@
import { styles } from './styles'

export default function Page() {
return (
<>
<h1>Page</h1>
<div id="cssm" className={styles.mod}>
CSSM
</div>
</>
)
}
4 changes: 4 additions & 0 deletions test/e2e/app-dir/app/app/css/css-page/unused/styles.js
@@ -0,0 +1,4 @@
import unused from './unused.module.css'
import styles from '../style.module.css'

export { unused, styles }
@@ -0,0 +1,3 @@
.this_should_not_be_included {
color: red;
}
11 changes: 11 additions & 0 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -1186,6 +1186,17 @@ describe('app dir', () => {
)
).toBe('rgb(0, 0, 255)')
})

if (!isDev) {
it('should not include unused css modules in the page in prod', async () => {
const browser = await webdriver(next.url, '/css/css-page')
expect(
await browser.eval(
`[...document.styleSheets].some(({ rules }) => [...rules].some(rule => rule.selectorText.includes('this_should_not_be_included')))`
)
).toBe(false)
})
}
})

describe('client layouts', () => {
Expand Down

0 comments on commit 76a8d49

Please sign in to comment.