Skip to content

Commit

Permalink
Ensure entry tracing applies for app correctly (vercel#41140)
Browse files Browse the repository at this point in the history
This ensures we properly detect and trace `app` dir entries and adds a
regression test to ensure this is working as expected.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see `contributing.md`
  • Loading branch information
ijjk authored and Kikobeats committed Oct 24, 2022
1 parent 9e8f99a commit 59df4b6
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/next/build/webpack-config.ts
Expand Up @@ -1817,8 +1817,8 @@ export default async function getBaseWebpackConfig(
{
appDir: dir,
esmExternals: config.experimental.esmExternals,
staticImageImports: !config.images.disableStaticImages,
outputFileTracingRoot: config.experimental.outputFileTracingRoot,
appDirEnabled: !!config.experimental.appDir,
}
),
// Moment.js is an extremely popular library that bundles large locale files
Expand Down
Expand Up @@ -24,7 +24,7 @@ const TRACE_IGNORES = [
function getModuleFromDependency(
compilation: any,
dep: any
): webpack.Module & { resource?: string } {
): webpack.Module & { resource?: string; request?: string } {
return compilation.moduleGraph.getModule(dep)
}

Expand Down Expand Up @@ -84,30 +84,30 @@ function getFilesMapFromReasons(

export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {
private appDir: string
private appDirEnabled?: boolean
private tracingRoot: string
private entryTraces: Map<string, Set<string>>
private excludeFiles: string[]
private esmExternals?: NextConfigComplete['experimental']['esmExternals']
private staticImageImports?: boolean

constructor({
appDir,
appDirEnabled,
excludeFiles,
esmExternals,
staticImageImports,
outputFileTracingRoot,
}: {
appDir: string
appDirEnabled?: boolean
excludeFiles?: string[]
staticImageImports: boolean
outputFileTracingRoot?: string
esmExternals?: NextConfigComplete['experimental']['esmExternals']
}) {
this.appDir = appDir
this.entryTraces = new Map()
this.esmExternals = esmExternals
this.appDirEnabled = appDirEnabled
this.excludeFiles = excludeFiles || []
this.staticImageImports = staticImageImports
this.tracingRoot = outputFileTracingRoot || appDir
}

Expand Down Expand Up @@ -257,15 +257,44 @@ export class TraceEntryPointsPlugin implements webpack.WebpackPluginInstance {

finishModulesSpan.traceChild('get-entries').traceFn(() => {
compilation.entries.forEach((entry, name) => {
if (name?.replace(/\\/g, '/').startsWith('pages/')) {
const normalizedName = name?.replace(/\\/g, '/')

const isPage = normalizedName.startsWith('pages/')
const isApp =
this.appDirEnabled && normalizedName.startsWith('app/')

if (isApp || isPage) {
for (const dep of entry.dependencies) {
if (!dep) continue
const entryMod = getModuleFromDependency(compilation, dep)

// since app entries are wrapped in next-app-loader
// we need to pull the original pagePath for
// referencing during tracing
if (isApp && entryMod.request) {
const loaderQueryIdx = entryMod.request.indexOf('?')

const loaderQuery = new URLSearchParams(
entryMod.request.substring(loaderQueryIdx)
)
const resource =
loaderQuery
.get('pagePath')
?.replace(
'private-next-app-dir',
nodePath.join(this.appDir, 'app')
) || ''

entryModMap.set(resource, entryMod)
entryNameMap.set(resource, name)
}

if (entryMod && entryMod.resource) {
if (
entryMod.resource.replace(/\\/g, '/').includes('pages/')
) {
const normalizedResource = entryMod.resource.replace(
/\\/g,
'/'
)
if (normalizedResource.includes('pages/')) {
entryNameMap.set(entryMod.resource, name)
entryModMap.set(entryMod.resource, entryMod)
} else {
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/app-dir/app/app/dashboard/deployments/[id]/data.json
@@ -0,0 +1,3 @@
{
"hello": "world"
}
9 changes: 9 additions & 0 deletions test/e2e/app-dir/app/app/dashboard/deployments/[id]/page.js
@@ -1,6 +1,15 @@
import { experimental_use as use } from 'react'
import fs from 'fs'
import path from 'path'

async function getData({ params }) {
const data = JSON.parse(
fs.readFileSync(
path.join(process.cwd(), 'app/dashboard/deployments/[id]/data.json')
)
)
console.log('data.json', data)

return {
id: params.id,
}
Expand Down
13 changes: 13 additions & 0 deletions test/e2e/app-dir/index.test.ts
Expand Up @@ -35,6 +35,19 @@ describe('app dir', () => {
})
afterAll(() => next.destroy())

if ((global as any).isNextStart) {
it('should generate build traces correctly', async () => {
const trace = JSON.parse(
await next.readFile(
'.next/server/app/dashboard/deployments/[id]/page.js.nft.json'
)
) as { files: string[] }
expect(trace.files.some((file) => file.endsWith('data.json'))).toBe(
true
)
})
}

it('should use application/octet-stream for flight', async () => {
const res = await fetchViaHTTP(
next.url,
Expand Down

0 comments on commit 59df4b6

Please sign in to comment.