Skip to content

Commit

Permalink
Move tracing next-server to next build (#30190)
Browse files Browse the repository at this point in the history
As discussed this moves tracing `next-server` into `next build` since the pre-trace at publish time isn't able to reliably give us file locations since `node_module` file locations can vary based on installation. This also adds caching the trace so that we only need to retrace `next-server` when a lockfile changes. 

A follow-up PR will add documentation for these traces explaining how they can be leveraged. 

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have 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 helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
  • Loading branch information
ijjk authored Oct 22, 2021
1 parent d5aa038 commit 7054096
Show file tree
Hide file tree
Showing 4 changed files with 237 additions and 63 deletions.
217 changes: 166 additions & 51 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1024,70 +1024,185 @@ 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<string[]> => {
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<string[]> => {
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)

for (let page of pageKeys) {
const includeGlobs = pageTraceIncludes.get(page)
const excludeGlobs = pageTraceExcludes.get(page)
page = normalizePagePath(page)
if (!includeGlobs?.length && !excludeGlobs?.length) {
return
}

if (!includeGlobs?.length && !excludeGlobs?.length) {
continue
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])

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)
}
})
}

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)).replace(/\\/g, '/')
)
})
}

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) {
Expand Down
3 changes: 1 addition & 2 deletions packages/next/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
40 changes: 40 additions & 0 deletions test/integration/production/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,46 @@ 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)

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)
expect(
serverTrace.files.some((file) => file.includes('react.development.js'))
).toBe(false)
}

const checks = [
{
page: '/_app',
Expand Down
40 changes: 30 additions & 10 deletions test/production/required-server-files.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -47,7 +48,10 @@ describe('should set-up next', () => {
})
await next.stop()
const keptFiles = new Set<string>()
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')
Expand Down Expand Up @@ -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'
) {
Expand Down Expand Up @@ -553,35 +561,47 @@ 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 () => {
errors = []
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 () => {
errors = []
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 () => {
errors = []
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 () => {
Expand Down

0 comments on commit 7054096

Please sign in to comment.