From 4202753c2fe2bfb162faba68be167b98f71f5c3f Mon Sep 17 00:00:00 2001 From: "jj@jjsweb.site" Date: Fri, 22 Oct 2021 15:22:45 -0500 Subject: [PATCH 1/4] Move tracing next-server to next build --- packages/next/build/index.ts | 215 +++++++++++++----- packages/next/package.json | 3 +- .../integration/production/test/index.test.js | 37 +++ test/production/required-server-files.test.ts | 40 +++- 4 files changed, 232 insertions(+), 63 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index f79777681e823..f257b1c325e48 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -1024,70 +1024,183 @@ export default async function build( } if (config.experimental.outputFileTracing) { - const globOrig = - require('next/dist/compiled/glob') as typeof import('next/dist/compiled/glob') - const glob = (pattern: string): Promise => { - return new Promise((resolve, reject) => { - globOrig(pattern, { cwd: dir }, (err, files) => { - if (err) { - return reject(err) - } - resolve(files) + const { nodeFileTrace } = + require('next/dist/compiled/@vercel/nft') as typeof import('next/dist/compiled/@vercel/nft') + + const includeExcludeSpan = nextBuildSpan.traceChild( + 'apply-include-excludes' + ) + + await includeExcludeSpan.traceAsyncFn(async () => { + const globOrig = + require('next/dist/compiled/glob') as typeof import('next/dist/compiled/glob') + const glob = (pattern: string): Promise => { + return new Promise((resolve, reject) => { + globOrig(pattern, { cwd: dir }, (err, files) => { + if (err) { + return reject(err) + } + resolve(files) + }) }) - }) - } + } + + for (let page of pageKeys) { + await includeExcludeSpan + .traceChild('include-exclude', { page }) + .traceAsyncFn(async () => { + const includeGlobs = pageTraceIncludes.get(page) + const excludeGlobs = pageTraceExcludes.get(page) + page = normalizePagePath(page) + + if (!includeGlobs?.length && !excludeGlobs?.length) { + return + } + + const traceFile = path.join( + distDir, + 'server/pages', + `${page}.js.nft.json` + ) + const pageDir = path.dirname(traceFile) + const traceContent = JSON.parse( + await promises.readFile(traceFile, 'utf8') + ) + let includes: string[] = [] + + if (includeGlobs?.length) { + for (const includeGlob of includeGlobs) { + const results = await glob(includeGlob) + includes.push( + ...results.map((file) => { + return path.relative(pageDir, path.join(dir, file)) + }) + ) + } + } + const combined = new Set([...traceContent.files, ...includes]) - for (let page of pageKeys) { - const includeGlobs = pageTraceIncludes.get(page) - const excludeGlobs = pageTraceExcludes.get(page) - page = normalizePagePath(page) + if (excludeGlobs?.length) { + const resolvedGlobs = excludeGlobs.map((exclude) => + path.join(dir, exclude) + ) + combined.forEach((file) => { + if (isMatch(path.join(pageDir, file), resolvedGlobs)) { + combined.delete(file) + } + }) + } - if (!includeGlobs?.length && !excludeGlobs?.length) { - continue + await promises.writeFile( + traceFile, + JSON.stringify({ + version: traceContent.version, + files: [...combined], + }) + ) + }) } + }) - const traceFile = path.join( - distDir, - 'server/pages', - `${page}.js.nft.json` - ) - const pageDir = path.dirname(traceFile) - const traceContent = JSON.parse( - await promises.readFile(traceFile, 'utf8') - ) - let includes: string[] = [] - - if (includeGlobs?.length) { - for (const includeGlob of includeGlobs) { - const results = await glob(includeGlob) - includes.push( - ...results.map((file) => { - return path.relative(pageDir, path.join(dir, file)) + // TODO: move this inside of webpack so it can be cached + // between builds. Should only need to be re-run on lockfile change + await nextBuildSpan + .traceChild('trace-next-server') + .traceAsyncFn(async () => { + let cacheKey: string | undefined + // consider all lockFiles in tree in case user accidentally + // has both package-lock.json and yarn.lock + const lockFiles: string[] = ( + await Promise.all( + ['package-lock.json', 'yarn.lock', 'pnpm-lock.yaml'].map((file) => + findUp(file, { cwd: dir }) + ) + ) + ).filter(Boolean) as any // TypeScript doesn't like this filter + + const nextServerTraceOutput = path.join( + distDir, + 'next-server.js.nft.json' + ) + const cachedTracePath = path.join( + distDir, + 'cache/next-server.js.nft.json' + ) + + if (lockFiles.length > 0) { + const cacheHash = ( + require('crypto') as typeof import('crypto') + ).createHash('sha256') + + cacheHash.update(require('next/package').version) + + await Promise.all( + lockFiles.map(async (lockFile) => { + cacheHash.update(await promises.readFile(lockFile)) }) ) + cacheKey = cacheHash.digest('hex') + + try { + const existingTrace = JSON.parse( + await promises.readFile(cachedTracePath, 'utf8') + ) + + if (existingTrace.cacheKey === cacheKey) { + await promises.copyFile(cachedTracePath, nextServerTraceOutput) + return + } + } catch (_) {} } - } - const combined = new Set([...traceContent.files, ...includes]) - if (excludeGlobs?.length) { - const resolvedGlobs = excludeGlobs.map((exclude) => - path.join(dir, exclude) + const root = path.parse(dir).root + const serverResult = await nodeFileTrace( + [require.resolve('next/dist/server/next-server')], + { + base: root, + processCwd: dir, + ignore: [ + '**/next/dist/pages/**/*', + '**/next/dist/server/image-optimizer.js', + '**/next/dist/compiled/@ampproject/toolbox-optimizer/**/*', + '**/next/dist/server/lib/squoosh/**/*.wasm', + '**/next/dist/compiled/webpack/(bundle4|bundle5).js', + '**/node_modules/react/**/*.development.js', + '**/node_modules/react-dom/**/*.development.js', + '**/node_modules/use-subscription/**/*.development.js', + '**/node_modules/sharp/**/*', + '**/node_modules/webpack5/**/*', + ], + } ) - combined.forEach((file) => { - if (isMatch(path.join(pageDir, file), resolvedGlobs)) { - combined.delete(file) + + const tracedFiles = new Set() + + serverResult.fileList.forEach((file) => { + const reason = serverResult.reasons.get(file) + + if (reason?.type === 'initial') { + return } + tracedFiles.add(path.relative(distDir, path.join(root, file))) }) - } - await promises.writeFile( - traceFile, - JSON.stringify({ - version: traceContent.version, - files: [...combined], - }) - ) - } + await promises.writeFile( + nextServerTraceOutput, + JSON.stringify({ + version: 1, + cacheKey, + files: [...tracedFiles], + } as { + version: number + files: string[] + }) + ) + await promises.unlink(cachedTracePath).catch(() => {}) + await promises + .copyFile(nextServerTraceOutput, cachedTracePath) + .catch(() => {}) + }) } if (serverPropsPages.size > 0 || ssgPages.size > 0) { diff --git a/packages/next/package.json b/packages/next/package.json index caea67a5ca9de..d8124a7bdd516 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -52,8 +52,7 @@ "scripts": { "dev": "taskr", "release": "taskr release", - "prepublish": "npm run release && yarn types && yarn trace-server", - "trace-server": "node ../../scripts/trace-next-server.js", + "prepublish": "npm run release && yarn types", "types": "tsc --declaration --emitDeclarationOnly --declarationDir dist", "typescript": "tsc --noEmit --declaration", "ncc-compiled": "ncc cache clean && taskr ncc", diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 57ee10fa05b67..74ea07d64b877 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -68,6 +68,43 @@ describe('Production Usage', () => { }) it('should output traces', async () => { + const serverTrace = await fs.readJSON( + join(appDir, '.next/next-server.js.nft.json') + ) + + expect(serverTrace.version).toBe(1) + expect( + serverTrace.files.some((file) => + file.includes('next/dist/server/send-payload.js') + ) + ).toBe(true) + expect( + serverTrace.files.some((file) => + file.includes('next/dist/server/normalize-page-path.js') + ) + ).toBe(true) + expect( + serverTrace.files.some((file) => + file.includes('next/dist/server/render.js') + ) + ).toBe(true) + expect( + serverTrace.files.some((file) => + file.includes('next/dist/server/load-components.js') + ) + ).toBe(true) + expect( + serverTrace.files.some((file) => + file.includes('next/dist/compiled/webpack/bundle5.js') + ) + ).toBe(false) + expect( + serverTrace.files.some((file) => file.includes('node_modules/sharp')) + ).toBe(false) + expect( + serverTrace.files.some((file) => file.includes('react.development.js')) + ).toBe(false) + const checks = [ { page: '/_app', diff --git a/test/production/required-server-files.test.ts b/test/production/required-server-files.test.ts index 34b6c5a23da89..fdc6219e2df2e 100644 --- a/test/production/required-server-files.test.ts +++ b/test/production/required-server-files.test.ts @@ -5,6 +5,7 @@ import { join, dirname } from 'path' import { createNext, FileRef } from 'e2e-utils' import { NextInstance } from 'test/lib/next-modes/base' import { + check, fetchViaHTTP, findPort, initNextServerScript, @@ -47,7 +48,10 @@ describe('should set-up next', () => { }) await next.stop() const keptFiles = new Set() - const nextServerTrace = require('next/dist/server/next-server.js.nft.json') + const nextServerTrace = require(join( + next.testDir, + '.next/next-server.js.nft.json' + )) requiredFilesManifest = JSON.parse( await next.readFile('.next/required-server-files.json') @@ -76,12 +80,16 @@ describe('should set-up next', () => { dot: true, }) + const nextServerTraceFiles = nextServerTrace.files.map((file) => { + return join(next.testDir, '.next/', file) + }) + for (const file of allFiles) { const filePath = join(next.testDir, file) if ( !keptFiles.has(file) && !(await _fs.stat(filePath).catch(() => null))?.isDirectory() && - !nextServerTrace.files.includes(file) && + !nextServerTraceFiles.includes(filePath) && !file.match(/node_modules\/(react|react-dom)\//) && file !== 'node_modules/next/dist/server/next-server.js' ) { @@ -553,8 +561,11 @@ describe('should set-up next', () => { const res = await fetchViaHTTP(appPort, '/errors/gip', { crash: '1' }) expect(res.status).toBe(500) expect(await res.text()).toBe('error') - expect(errors.length).toBe(1) - expect(errors[0]).toContain('gip hit an oops') + + await check( + () => (errors[0].includes('gip hit an oops') ? 'success' : errors[0]), + 'success' + ) }) it('should bubble error correctly for gssp page', async () => { @@ -562,8 +573,10 @@ describe('should set-up next', () => { const res = await fetchViaHTTP(appPort, '/errors/gssp', { crash: '1' }) expect(res.status).toBe(500) expect(await res.text()).toBe('error') - expect(errors.length).toBe(1) - expect(errors[0]).toContain('gssp hit an oops') + await check( + () => (errors[0].includes('gssp hit an oops') ? 'success' : errors[0]), + 'success' + ) }) it('should bubble error correctly for gsp page', async () => { @@ -571,8 +584,10 @@ describe('should set-up next', () => { const res = await fetchViaHTTP(appPort, '/errors/gsp/crash') expect(res.status).toBe(500) expect(await res.text()).toBe('error') - expect(errors.length).toBe(1) - expect(errors[0]).toContain('gsp hit an oops') + await check( + () => (errors[0].includes('gsp hit an oops') ? 'success' : errors[0]), + 'success' + ) }) it('should bubble error correctly for API page', async () => { @@ -580,8 +595,13 @@ describe('should set-up next', () => { const res = await fetchViaHTTP(appPort, '/api/error') expect(res.status).toBe(500) expect(await res.text()).toBe('error') - expect(errors.length).toBe(1) - expect(errors[0]).toContain('some error from /api/error') + await check( + () => + errors[0].includes('some error from /api/error') + ? 'success' + : errors[0], + 'success' + ) }) it('should normalize optional values correctly for SSP page', async () => { From 0b1d6de3a06c513a450f22847c6d3dd6171e48ed Mon Sep 17 00:00:00 2001 From: "jj@jjsweb.site" Date: Fri, 22 Oct 2021 17:02:33 -0500 Subject: [PATCH 2/4] normalize windows slashes --- packages/next/build/index.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index f257b1c325e48..0c1af41b6cc1f 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -1182,7 +1182,9 @@ export default async function build( if (reason?.type === 'initial') { return } - tracedFiles.add(path.relative(distDir, path.join(root, file))) + tracedFiles.add( + path.relative(distDir, path.join(root, file)).replace(/\\/g, '/') + ) }) await promises.writeFile( From e8cb694eac6fd25a264febc7625eb6ef3cd79c11 Mon Sep 17 00:00:00 2001 From: "jj@jjsweb.site" Date: Fri, 22 Oct 2021 17:32:10 -0500 Subject: [PATCH 3/4] Update test --- test/integration/production/test/index.test.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 74ea07d64b877..1f14d3ac0dfd1 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -1,5 +1,6 @@ /* eslint-env jest */ /* global browserName */ +import os from 'os' import cheerio from 'cheerio' import fs, { existsSync } from 'fs-extra' import globOriginal from 'glob' @@ -98,12 +99,15 @@ describe('Production Usage', () => { file.includes('next/dist/compiled/webpack/bundle5.js') ) ).toBe(false) - expect( - serverTrace.files.some((file) => file.includes('node_modules/sharp')) - ).toBe(false) - expect( - serverTrace.files.some((file) => file.includes('react.development.js')) - ).toBe(false) + + if (os.platform() !== 'win32') { + expect( + serverTrace.files.some((file) => file.includes('node_modules/sharp')) + ).toBe(false) + expect( + serverTrace.files.some((file) => file.includes('react.development.js')) + ).toBe(false) + } const checks = [ { From 37ad7502f9b94f243cb9b9530e17ecbd4535a478 Mon Sep 17 00:00:00 2001 From: "jj@jjsweb.site" Date: Fri, 22 Oct 2021 18:11:58 -0500 Subject: [PATCH 4/4] apply suggestions --- test/integration/production/test/index.test.js | 13 ++++++------- test/production/required-server-files.test.ts | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js index 1f14d3ac0dfd1..51683cd6139fd 100644 --- a/test/integration/production/test/index.test.js +++ b/test/integration/production/test/index.test.js @@ -1,6 +1,5 @@ /* eslint-env jest */ /* global browserName */ -import os from 'os' import cheerio from 'cheerio' import fs, { existsSync } from 'fs-extra' import globOriginal from 'glob' @@ -94,13 +93,13 @@ describe('Production Usage', () => { file.includes('next/dist/server/load-components.js') ) ).toBe(true) - expect( - serverTrace.files.some((file) => - file.includes('next/dist/compiled/webpack/bundle5.js') - ) - ).toBe(false) - if (os.platform() !== 'win32') { + if (process.platform !== 'win32') { + expect( + serverTrace.files.some((file) => + file.includes('next/dist/compiled/webpack/bundle5.js') + ) + ).toBe(false) expect( serverTrace.files.some((file) => file.includes('node_modules/sharp')) ).toBe(false) diff --git a/test/production/required-server-files.test.ts b/test/production/required-server-files.test.ts index fdc6219e2df2e..3dc8e6287d410 100644 --- a/test/production/required-server-files.test.ts +++ b/test/production/required-server-files.test.ts @@ -81,7 +81,7 @@ describe('should set-up next', () => { }) const nextServerTraceFiles = nextServerTrace.files.map((file) => { - return join(next.testDir, '.next/', file) + return join(next.testDir, '.next', file) }) for (const file of allFiles) {