Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure entry tracing applies for app correctly #41140

Merged
merged 1 commit into from Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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,
ijjk marked this conversation as resolved.
Show resolved Hide resolved
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(
styfle marked this conversation as resolved.
Show resolved Hide resolved
entryMod.request.substring(loaderQueryIdx)
)
const resource =
loaderQuery
.get('pagePath')
?.replace(
styfle marked this conversation as resolved.
Show resolved Hide resolved
'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
@@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could probably remove this console.log now


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