Skip to content

Commit

Permalink
Fix Shared Sizes Missing Commons (#9752)
Browse files Browse the repository at this point in the history
* Fix shared sizes missing commons

* Add tests
  • Loading branch information
ijjk authored and Timer committed Dec 14, 2019
1 parent eee4efd commit a32af59
Show file tree
Hide file tree
Showing 6 changed files with 70 additions and 6 deletions.
5 changes: 4 additions & 1 deletion packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,9 @@ export default async function build(dir: string, conf = null): Promise<void> {
bundleRelative
)

let isStatic = false
let isSsg = false
let isStatic = false
let isHybridAmp = false
let ssgPageRoutes: string[] | null = null

pagesManifest[page] = bundleRelative.replace(/\\/g, '/')
Expand Down Expand Up @@ -430,6 +431,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
)

if (result.isHybridAmp) {
isHybridAmp = true
hybridAmpPages.add(page)
}

Expand All @@ -456,6 +458,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
serverBundle,
static: isStatic,
isSsg,
isHybridAmp,
ssgPageRoutes,
})
})
Expand Down
35 changes: 30 additions & 5 deletions packages/next/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export function collectPages(

export interface PageInfo {
isAmp?: boolean
isHybridAmp?: boolean
size: number
static: boolean
isSsg: boolean
Expand Down Expand Up @@ -135,7 +136,12 @@ export async function printTreeView(
}
})

const sharedData = await getSharedSizes(distPath, buildManifest, isModern)
const sharedData = await getSharedSizes(
distPath,
buildManifest,
isModern,
pageInfos
)

messages.push(['+ shared by all', getPrettySize(sharedData.total)])
Object.keys(sharedData.files)
Expand Down Expand Up @@ -255,15 +261,18 @@ let cachedBuildManifest: BuildManifestShape | undefined

let lastCompute: ComputeManifestShape | undefined
let lastComputeModern: boolean | undefined
let lastComputePageInfo: boolean | undefined

async function computeFromManifest(
manifest: BuildManifestShape,
distPath: string,
isModern: boolean
isModern: boolean,
pageInfos?: Map<string, PageInfo>
): Promise<ComputeManifestShape> {
if (
Object.is(cachedBuildManifest, manifest) &&
lastComputeModern === isModern
lastComputeModern === isModern &&
lastComputePageInfo === !!pageInfos
) {
return lastCompute!
}
Expand All @@ -275,6 +284,15 @@ async function computeFromManifest(
return
}

if (pageInfos) {
const cleanKey = key.replace(/\/index$/, '') || '/'
const pageInfo = pageInfos.get(cleanKey)
// don't include AMP pages since they don't rely on shared bundles
if (pageInfo?.isHybridAmp || pageInfo?.isAmp) {
return
}
}

++expected
manifest.pages[key].forEach(file => {
if (
Expand Down Expand Up @@ -321,6 +339,7 @@ async function computeFromManifest(

cachedBuildManifest = manifest
lastComputeModern = isModern
lastComputePageInfo = !!pageInfos
return lastCompute!
}

Expand All @@ -333,9 +352,15 @@ function difference<T>(main: T[], sub: T[]): T[] {
export async function getSharedSizes(
distPath: string,
buildManifest: BuildManifestShape,
isModern: boolean
isModern: boolean,
pageInfos: Map<string, PageInfo>
): Promise<{ total: number; files: { [page: string]: number } }> {
const data = await computeFromManifest(buildManifest, distPath, isModern)
const data = await computeFromManifest(
buildManifest,
distPath,
isModern,
pageInfos
)
return { total: data.sizeCommonFiles, files: data.sizeCommonFile }
}

Expand Down
2 changes: 2 additions & 0 deletions test/integration/build-output/fixtures/with-amp/pages/amp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const config = { amp: true }
export default () => 'hi'
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const config = { amp: 'hybrid' }
export default () => 'hi'
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function() {
return <div />
}
29 changes: 29 additions & 0 deletions test/integration/build-output/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ describe('Build Output', () => {
expect(stdout).toMatch(/\/ [ ]* \d{1,} B/)
expect(stdout).toMatch(/\+ shared by all [ 0-9.]* kB/)
expect(stdout).toMatch(/ runtime\/main\.js [ 0-9.]* kB/)
expect(stdout).toMatch(/ chunks\/commons\.js [ 0-9. ]* kB/)

expect(stdout).not.toContain('/_document')
expect(stdout).not.toContain('/_app')
Expand All @@ -50,6 +51,7 @@ describe('Build Output', () => {
expect(stdout).toMatch(/\/_app [ ]* \d{1,} B/)
expect(stdout).toMatch(/\+ shared by all [ 0-9.]* kB/)
expect(stdout).toMatch(/ runtime\/main\.js [ 0-9.]* kB/)
expect(stdout).toMatch(/ chunks\/commons\.js [ 0-9. ]* kB/)

expect(stdout).not.toContain('/_document')
expect(stdout).not.toContain('/_error')
Expand All @@ -59,6 +61,32 @@ describe('Build Output', () => {
})
})

describe('With AMP Output', () => {
const appDir = join(fixturesDir, 'with-amp')

beforeAll(async () => {
await remove(join(appDir, '.next'))
})

it('should not include custom error', async () => {
const { stdout } = await nextBuild(appDir, [], {
stdout: true,
})

expect(stdout).toMatch(/\/ [ 0-9.]* kB/)
expect(stdout).toMatch(/\/amp .* AMP/)
expect(stdout).toMatch(/\/hybrid [ 0-9.]* B/)
expect(stdout).toMatch(/\+ shared by all [ 0-9.]* kB/)
expect(stdout).toMatch(/ runtime\/main\.js [ 0-9.]* kB/)
expect(stdout).toMatch(/ chunks\/commons\.js [ 0-9. ]* kB/)

expect(stdout).not.toContain('/_document')
expect(stdout).not.toContain('/_error')

expect(stdout).toContain('○ /')
})
})

describe('Custom Error Output', () => {
const appDir = join(fixturesDir, 'with-error')

Expand All @@ -75,6 +103,7 @@ describe('Build Output', () => {
expect(stdout).toMatch(/λ \/_error [ ]* \d{1,} B/)
expect(stdout).toMatch(/\+ shared by all [ 0-9.]* kB/)
expect(stdout).toMatch(/ runtime\/main\.js [ 0-9.]* kB/)
expect(stdout).toMatch(/ chunks\/commons\.js [ 0-9. ]* kB/)

expect(stdout).not.toContain('/_document')
expect(stdout).not.toContain('/_app')
Expand Down

0 comments on commit a32af59

Please sign in to comment.