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

Ensure all CSS files are included for experimental critical CSS #33752

Merged
merged 3 commits into from Jan 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 15 additions & 4 deletions packages/next/build/index.ts
Expand Up @@ -9,7 +9,7 @@ import { escapeStringRegexp } from '../shared/lib/escape-regexp'
import findUp from 'next/dist/compiled/find-up'
import { nanoid } from 'next/dist/compiled/nanoid/index.cjs'
import { pathToRegexp } from 'next/dist/compiled/path-to-regexp'
import path from 'path'
import path, { join } from 'path'
import formatWebpackMessages from '../client/dev/error-overlay/format-webpack-messages'
import {
STATIC_STATUS_PAGE_GET_INITIAL_PROPS_ERROR,
Expand Down Expand Up @@ -89,7 +89,6 @@ import {
PageInfo,
printCustomRoutes,
printTreeView,
getCssFilePaths,
getUnresolvedModuleFromError,
copyTracedFiles,
isReservedPage,
Expand Down Expand Up @@ -1364,10 +1363,22 @@ export default async function build(
await writeBuildId(distDir, buildId)

if (config.experimental.optimizeCss) {
const cssFilePaths = getCssFilePaths(buildManifest)
const globOrig =
require('next/dist/compiled/glob') as typeof import('next/dist/compiled/glob')

const cssFilePaths = await new Promise<string[]>((resolve, reject) => {
globOrig('**/*.css', { cwd: join(distDir, 'static') }, (err, files) => {
if (err) {
return reject(err)
}
resolve(files)
})
})

requiredServerFiles.files.push(
...cssFilePaths.map((filePath) => path.join(config.distDir, filePath))
...cssFilePaths.map((filePath) =>
path.join(config.distDir, 'static', filePath)
)
)
}

Expand Down
13 changes: 0 additions & 13 deletions packages/next/build/utils.ts
Expand Up @@ -1103,19 +1103,6 @@ export function detectConflictingPaths(
}
}

export function getCssFilePaths(buildManifest: BuildManifest): string[] {
const cssFiles = new Set<string>()
Object.values(buildManifest.pages).forEach((files) => {
files.forEach((file) => {
if (file.endsWith('.css')) {
cssFiles.add(file)
}
})
})

return [...cssFiles]
}

export function getRawPageExtensions(pageExtensions: string[]): string[] {
return pageExtensions.filter(
(ext) => !ext.startsWith('client.') && !ext.startsWith('server.')
Expand Down
5 changes: 5 additions & 0 deletions test/integration/critical-css/components/hello.js
@@ -0,0 +1,5 @@
import styles from './hello.module.css'

export default function Hello() {
return <p className={styles.hello}>hello world</p>
}
3 changes: 3 additions & 0 deletions test/integration/critical-css/components/hello.module.css
@@ -0,0 +1,3 @@
.hello {
color: orange;
}
21 changes: 21 additions & 0 deletions test/integration/critical-css/pages/another.js
@@ -0,0 +1,21 @@
import styles from '../styles/index.module.css'
import dynamic from 'next/dynamic'

const Hello = dynamic(() => import('../components/hello'))

export default function Home() {
return (
<div className={styles.hello}>
<p>Hello World</p>
<Hello />
</div>
)
}

export const getServerSideProps = () => {
return {
props: {
hello: 'world',
},
}
}
2 changes: 2 additions & 0 deletions test/integration/critical-css/pages/index.js
@@ -1,9 +1,11 @@
import styles from '../styles/index.module.css'
import Hello from '../components/hello'

export default function Home() {
return (
<div className={styles.hello}>
<p>Hello World</p>
<Hello />
</div>
)
}
Expand Down
46 changes: 30 additions & 16 deletions test/integration/critical-css/test/index.test.js
@@ -1,5 +1,6 @@
/* eslint-env jest */

import globOrigig from 'glob'
import { promisify } from 'util'
import { join } from 'path'
import {
killApp,
Expand All @@ -10,12 +11,29 @@ import {
} from 'next-test-utils'
import fs from 'fs-extra'

const glob = promisify(globOrigig)
const appDir = join(__dirname, '../')
const nextConfig = join(appDir, 'next.config.js')
let appPort
let app

function runTests() {
it('should have all CSS files in manifest', async () => {
const cssFiles = (
await glob('**/*.css', {
cwd: join(appDir, '.next/static'),
})
).map((file) => join('.next/static', file))

const requiredServerFiles = await fs.readJSON(
join(appDir, '.next/required-server-files.json')
)

expect(
requiredServerFiles.files.filter((file) => file.endsWith('.css'))
).toEqual(cssFiles)
})

it('should inline critical CSS', async () => {
const html = await renderViaHTTP(appPort, '/')
expect(html).toMatch(
Expand All @@ -24,6 +42,14 @@ function runTests() {
expect(html).toMatch(/body{font-family:SF Pro Text/)
})

it('should inline critical CSS (dynamic)', async () => {
const html = await renderViaHTTP(appPort, '/another')
expect(html).toMatch(
/<link rel="stylesheet" href="\/_next\/static\/css\/.*\.css" .*>/
)
expect(html).toMatch(/body{font-family:SF Pro Text/)
})

it('should not inline non-critical css', async () => {
const html = await renderViaHTTP(appPort, '/')
expect(html).not.toMatch(/.extra-style/)
Expand All @@ -45,22 +71,10 @@ describe('CSS optimization for SSR apps', () => {
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))
runTests()
})

describe('CSS optimization for serverless apps', () => {
beforeAll(async () => {
await fs.writeFile(
nextConfig,
`module.exports = { target: 'serverless', experimental: {optimizeCss: true} }`,
'utf8'
)
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
afterAll(async () => {
await killApp(app)
await fs.remove(nextConfig)
})
afterAll(() => killApp(app))
runTests()
})

Expand Down