Skip to content

Commit

Permalink
Separate Low Priority Files from Main Files (#10756)
Browse files Browse the repository at this point in the history
* Separate Low Priority Files from Main Files

* Fix tests
  • Loading branch information
Timer committed Feb 29, 2020
1 parent 5a82812 commit e608c86
Show file tree
Hide file tree
Showing 6 changed files with 40 additions and 38 deletions.
22 changes: 9 additions & 13 deletions packages/next/build/webpack/plugins/build-manifest-plugin.ts
@@ -1,7 +1,6 @@
import devalue from 'devalue'
import { Compiler } from 'webpack'
import { RawSource } from 'webpack-sources'

import {
BUILD_MANIFEST,
CLIENT_STATIC_FILES_PATH,
Expand All @@ -10,19 +9,12 @@ import {
IS_BUNDLED_PAGE_REGEX,
ROUTE_NAME_REGEX,
} from '../../../next-server/lib/constants'

interface AssetMap {
devFiles: string[]
pages: {
'/_app': string[]
[s: string]: string[]
}
}
import { BuildManifest } from '../../../next-server/server/get-page-files'

// This function takes the asset map generated in BuildManifestPlugin and creates a
// reduced version to send to the client.
const generateClientManifest = (
assetMap: AssetMap,
assetMap: BuildManifest,
isModern: boolean
): string => {
const clientManifest: { [s: string]: string[] } = {}
Expand Down Expand Up @@ -68,7 +60,11 @@ export default class BuildManifestPlugin {
'NextJsBuildManifest',
(compilation, callback) => {
const { chunks } = compilation
const assetMap: AssetMap = { devFiles: [], pages: { '/_app': [] } }
const assetMap: BuildManifest = {
devFiles: [],
lowPriorityFiles: [],
pages: { '/_app': [] },
}

const mainJsChunk = chunks.find(
c => c.name === CLIENT_STATIC_FILES_RUNTIME_MAIN
Expand Down Expand Up @@ -141,11 +137,11 @@ export default class BuildManifestPlugin {
// as a dependency for the app. If the flag is false, the file won't be
// downloaded by the client.
if (this.clientManifest) {
assetMap.pages['/_app'].push(
assetMap.lowPriorityFiles.push(
`${CLIENT_STATIC_FILES_PATH}/${this.buildId}/_buildManifest.js`
)
if (this.modern) {
assetMap.pages['/_app'].push(
assetMap.lowPriorityFiles.push(
`${CLIENT_STATIC_FILES_PATH}/${this.buildId}/_buildManifest.module.js`
)
}
Expand Down
1 change: 1 addition & 0 deletions packages/next/next-server/lib/utils.ts
Expand Up @@ -153,6 +153,7 @@ export type DocumentProps = DocumentInitialProps & {
hasCssMode: boolean
devFiles: string[]
files: string[]
lowPriorityFiles: string[]
polyfillFiles: string[]
dynamicImports: ManifestItem[]
assetPrefix?: string
Expand Down
2 changes: 2 additions & 0 deletions packages/next/next-server/server/get-page-files.ts
Expand Up @@ -2,7 +2,9 @@ import { normalizePagePath } from './normalize-page-path'

export type BuildManifest = {
devFiles: string[]
lowPriorityFiles: string[]
pages: {
'/_app': string[]
[page: string]: string[]
}
}
Expand Down
9 changes: 7 additions & 2 deletions packages/next/next-server/server/render.tsx
Expand Up @@ -174,6 +174,7 @@ function renderDocument(
staticMarkup,
devFiles,
files,
lowPriorityFiles,
polyfillFiles,
dynamicImports,
htmlProps,
Expand All @@ -191,8 +192,9 @@ function renderDocument(
hybridAmp: boolean
dynamicImportsIds: string[]
dynamicImports: ManifestItem[]
files: string[]
devFiles: string[]
files: string[]
lowPriorityFiles: string[]
polyfillFiles: string[]
htmlProps: any
bodyTags: any
Expand Down Expand Up @@ -229,6 +231,7 @@ function renderDocument(
staticMarkup,
devFiles,
files,
lowPriorityFiles,
polyfillFiles,
dynamicImports,
assetPrefix,
Expand Down Expand Up @@ -579,6 +582,7 @@ export async function renderToHTML(
...getPageFiles(buildManifest, pathname),
]),
]
const lowPriorityFiles = buildManifest.lowPriorityFiles
const polyfillFiles = getPageFiles(buildManifest, '/_polyfills')

const renderElementToString = staticMarkup
Expand Down Expand Up @@ -674,8 +678,9 @@ export async function renderToHTML(
hybridAmp,
dynamicImportsIds,
dynamicImports,
files,
devFiles,
files,
lowPriorityFiles,
polyfillFiles,
})

Expand Down
25 changes: 5 additions & 20 deletions packages/next/pages/_document.tsx
Expand Up @@ -44,10 +44,6 @@ function getOptionalModernScriptVariant(path: string) {
return path
}

function isLowPriority(file: string) {
return file.includes('_buildManifest')
}

/**
* `Document` component handles the initial `document` markup and renders only on the server side.
* Commonly used for implementing server side rendering for `css-in-js` libraries.
Expand Down Expand Up @@ -260,13 +256,7 @@ export class Head extends Component<
// `dynamicImports` will contain both `.js` and `.module.js` when
// the feature is enabled. This clause will filter down to the
// modern variants only.
//
// Also filter out low priority files because they should not be
// preloaded for performance reasons.
return (
file.endsWith(getOptionalModernScriptVariant('.js')) &&
!isLowPriority(file)
)
return file.endsWith(getOptionalModernScriptVariant('.js'))
})
: []

Expand Down Expand Up @@ -589,17 +579,12 @@ export class NextScript extends Component<OriginProps> {
}

getScripts() {
const { assetPrefix, files } = this.context._documentProps
if (!files || files.length === 0) {
return null
}
const { assetPrefix, files, lowPriorityFiles } = this.context._documentProps
const { _devOnlyInvalidateCacheQueryString } = this.context

const normalScripts = files.filter(
file => file.endsWith('.js') && !isLowPriority(file)
)
const lowPriorityScripts = files.filter(
file => file.endsWith('.js') && isLowPriority(file)
const normalScripts = files?.filter(file => file.endsWith('.js'))
const lowPriorityScripts = lowPriorityFiles?.filter(file =>
file.endsWith('.js')
)

return [...normalScripts, ...lowPriorityScripts].map(file => {
Expand Down
19 changes: 16 additions & 3 deletions test/integration/chunking/test/index.test.js
@@ -1,12 +1,12 @@
/* eslint-env jest */
/* global jasmine */
import { join } from 'path'
import cheerio from 'cheerio'
import express from 'express'
import { access, readdir, readFile, unlink } from 'fs-extra'
import http from 'http'
import { nextBuild, nextServer, promiseCall, stopApp } from 'next-test-utils'
import { readdir, readFile, unlink, access } from 'fs-extra'
import cheerio from 'cheerio'
import webdriver from 'next-webdriver'
import { join } from 'path'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1

Expand Down Expand Up @@ -90,6 +90,19 @@ describe('Chunking', () => {
).toBe(false)
})

it('should execute the build manifest', async () => {
const indexPage = await readFile(
join(appDir, '.next', 'server', 'static', buildId, 'pages', 'index.html')
)

const $ = cheerio.load(indexPage)
expect(
Array.from($('script'))
.map(e => e.attribs.src)
.some(entry => entry && entry.includes('_buildManifest'))
).toBe(true)
})

it('should not include more than one instance of react-dom', async () => {
const misplacedReactDom = stats.chunks.some(chunk => {
if (chunk.names.includes('framework')) {
Expand Down

0 comments on commit e608c86

Please sign in to comment.