Skip to content

Commit

Permalink
Update to filter loader specific files from traces (#32267)
Browse files Browse the repository at this point in the history
* Update to filter loader specific files from traces

* remove debug log
  • Loading branch information
ijjk committed Dec 14, 2021
1 parent 797e260 commit 2a5c21b
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 51 deletions.
Expand Up @@ -10,7 +10,6 @@ import { TRACE_OUTPUT_VERSION } from '../../../shared/lib/constants'
import { webpack, sources } from 'next/dist/compiled/webpack/webpack'
import type { webpack5 } from 'next/dist/compiled/webpack/webpack'
import {
nextImageLoaderRegex,
NODE_ESM_RESOLVE_OPTIONS,
NODE_RESOLVE_OPTIONS,
resolveExternal,
Expand All @@ -32,7 +31,8 @@ function getModuleFromDependency(

function getFilesMapFromReasons(
fileList: Set<string>,
reasons: NodeFileTraceReasons
reasons: NodeFileTraceReasons,
ignoreFn?: (file: string, parent?: string) => Boolean
) {
// this uses the reasons tree to collect files specific to a
// certain parent allowing us to not have to trace each parent
Expand All @@ -53,7 +53,10 @@ function getFilesMapFromReasons(
parentFiles = new Set()
parentFilesMap.set(parent, parentFiles)
}
parentFiles.add(file)

if (!ignoreFn?.(file, parent)) {
parentFiles.add(file)
}
const parentReason = reasons.get(parent)

if (parentReason?.parents) {
Expand Down Expand Up @@ -202,9 +205,11 @@ export class TraceEntryPointsPlugin implements webpack5.WebpackPluginInstance {
JSON.stringify({
version: TRACE_OUTPUT_VERSION,
files: [
...entryFiles,
...allEntryFiles,
...(this.entryTraces.get(entrypoint.name) || []),
...new Set([
...entryFiles,
...allEntryFiles,
...(this.entryTraces.get(entrypoint.name) || []),
]),
].map((file) => {
return nodePath
.relative(traceOutputPath, file)
Expand Down Expand Up @@ -349,7 +354,19 @@ export class TraceEntryPointsPlugin implements webpack5.WebpackPluginInstance {
await finishModulesSpan
.traceChild('collect-traced-files')
.traceAsyncFn(() => {
const parentFilesMap = getFilesMapFromReasons(fileList, reasons)
const parentFilesMap = getFilesMapFromReasons(
fileList,
reasons,
(file) => {
file = nodePath.join(this.tracingRoot, file)
const depMod = depModMap.get(file)

return (
Array.isArray(depMod?.loaders) &&
depMod.loaders.length > 0
)
}
)
entryPaths.forEach((entry) => {
const entryName = entryNameMap.get(entry)!
const normalizedEntry = nodePath.relative(
Expand Down Expand Up @@ -455,6 +472,7 @@ export class TraceEntryPointsPlugin implements webpack5.WebpackPluginInstance {
.catch((err) => callback(err))
}
)

let resolver = compilation.resolverFactory.get('normal')

function getPkgName(name: string) {
Expand Down Expand Up @@ -491,6 +509,12 @@ export class TraceEntryPointsPlugin implements webpack5.WebpackPluginInstance {
return reject(new Error('module not found'))
}

// webpack resolver doesn't strip loader query info
// from the result so use path instead
if (result.includes('?') || result.includes('!')) {
result = resContext?.path || result
}

try {
// we need to collect all parent package.json's used
// as webpack's resolve doesn't expose this and parent
Expand Down Expand Up @@ -570,11 +594,6 @@ export class TraceEntryPointsPlugin implements webpack5.WebpackPluginInstance {
job: import('@vercel/nft/out/node-file-trace').Job,
isEsmRequested: boolean
): Promise<string> => {
if (this.staticImageImports && nextImageLoaderRegex.test(request)) {
throw new Error(
`not resolving ${request} as this is handled by next-image-loader`
)
}
const context = nodePath.dirname(parent)
// When in esm externals mode, and using import, we resolve with
// ESM resolving options.
Expand Down
2 changes: 1 addition & 1 deletion test/integration/jsconfig-baseurl/test/index.test.js
Expand Up @@ -79,7 +79,7 @@ describe('TypeScript Features', () => {
).toBe(true)
expect(
helloTrace.files.some((file) => file.includes('components/world.js'))
).toBe(true)
).toBe(false)
expect(
helloTrace.files.some((file) => file.includes('react/index.js'))
).toBe(true)
Expand Down
4 changes: 4 additions & 0 deletions test/integration/jsconfig-paths/lib/a/api.js
@@ -1 +1,5 @@
import data from 'mypackage/data'

console.log(data)

export default () => 'Hello from a'

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 13 additions & 3 deletions test/integration/jsconfig-paths/test/index.test.js
Expand Up @@ -110,25 +110,35 @@ describe('TypeScript Features', () => {
singleAliasTrace.files.some((file) =>
file.includes('components/hello.js')
)
).toBe(true)
).toBe(false)
expect(
wildcardAliasTrace.files.some((file) =>
file.includes('mypackage/myfile.js')
)
).toBe(true)
expect(
wildcardAliasTrace.files.some((file) =>
file.includes('mypackage/data.js')
)
).toBe(false)
expect(
resolveOrderTrace.files.some((file) => file.includes('lib/a/api.js'))
).toBe(false)
expect(
resolveOrderTrace.files.some((file) =>
file.includes('mypackage/data.js')
)
).toBe(true)
expect(
resolveFallbackTrace.files.some((file) =>
file.includes('lib/b/b-only.js')
)
).toBe(true)
).toBe(false)
expect(
basicAliasTrace.files.some((file) =>
file.includes('components/world.js')
)
).toBe(true)
).toBe(false)
})
})
})
3 changes: 3 additions & 0 deletions test/integration/production-swcminify/pages/another.js
@@ -1,5 +1,8 @@
import url from 'url'
import Link from 'next/link'

console.log(url.parse('https://example.com'))

export default () => (
<div>
<Link href="/">
Expand Down
6 changes: 5 additions & 1 deletion test/integration/production-swcminify/pages/index.js
@@ -1,6 +1,10 @@
import Link from 'next/link'
import Link from 'next/link?loaderQuery'

if (typeof window === 'undefined') {
try {
let file = 'clear.js'
require('es5-ext/array/#/' + file)
} catch (_) {}
import('nanoid').then((mod) => console.log(mod.nanoid()))
}

Expand Down
53 changes: 36 additions & 17 deletions test/integration/production-swcminify/test/index.test.js
Expand Up @@ -103,9 +103,6 @@ describe.skip('Production Usage with swcMinify', () => {
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 = [
Expand All @@ -117,7 +114,7 @@ describe.skip('Production Usage with swcMinify', () => {
/node_modules\/react\/package\.json/,
/node_modules\/react\/cjs\/react\.production\.min\.js/,
],
notTests: [],
notTests: [/\0/, /\?/, /!/],
},
{
page: '/client-error',
Expand All @@ -127,13 +124,12 @@ describe.skip('Production Usage with swcMinify', () => {
/node_modules\/react\/index\.js/,
/node_modules\/react\/package\.json/,
/node_modules\/react\/cjs\/react\.production\.min\.js/,
/node_modules\/next/,
/next\/link\.js/,
/next\/dist\/client\/link\.js/,
/next\/dist\/shared\/lib\/router\/utils\/resolve-rewrites\.js/,
/next\/dist\/pages\/_error\.js/,
/next\/error\.js/,
],
notTests: [],
notTests: [/\0/, /\?/, /!/],
},
{
page: '/dynamic',
Expand All @@ -143,11 +139,11 @@ describe.skip('Production Usage with swcMinify', () => {
/node_modules\/react\/index\.js/,
/node_modules\/react\/package\.json/,
/node_modules\/react\/cjs\/react\.production\.min\.js/,
/node_modules\/next/,
/next\/link\.js/,
/next\/dist\/client\/link\.js/,
/next\/dist\/shared\/lib\/router\/utils\/resolve-rewrites\.js/,
],
notTests: [],
notTests: [/\0/, /\?/, /!/],
},
{
page: '/index',
Expand All @@ -157,16 +153,19 @@ describe.skip('Production Usage with swcMinify', () => {
/node_modules\/react\/index\.js/,
/node_modules\/react\/package\.json/,
/node_modules\/react\/cjs\/react\.production\.min\.js/,
/node_modules\/next/,
/next\/link\.js/,
/next\/dist\/client\/link\.js/,
/next\/dist\/shared\/lib\/router\/utils\/resolve-rewrites\.js/,
/node_modules\/nanoid\/index\.js/,
/node_modules\/nanoid\/url-alphabet\/index\.js/,
/node_modules\/es5-ext\/array\/#\/clear\.js/,
],
notTests: [
/node_modules\/nanoid\/index\.cjs/,
/next\/dist\/pages\/_error\.js/,
/next\/error\.js/,
/\0/,
/\?/,
/!/,
],
},
{
Expand All @@ -177,11 +176,12 @@ describe.skip('Production Usage with swcMinify', () => {
/node_modules\/react\/index\.js/,
/node_modules\/react\/package\.json/,
/node_modules\/react\/cjs\/react\.production\.min\.js/,
/node_modules\/react\/cjs\/react\.development\.js/,
/node_modules\/next/,
/next\/router\.js/,
/next\/dist\/client\/router\.js/,
/next\/dist\/shared\/lib\/router\/utils\/resolve-rewrites\.js/,
],
notTests: [],
notTests: [/\0/, /\?/, /!/],
},
{
page: '/next-import',
Expand All @@ -191,11 +191,17 @@ describe.skip('Production Usage with swcMinify', () => {
/node_modules\/react\/index\.js/,
/node_modules\/react\/package\.json/,
/node_modules\/react\/cjs\/react\.production\.min\.js/,
/node_modules\/next/,
/next\/link\.js/,
/next\/dist\/client\/link\.js/,
/next\/dist\/shared\/lib\/router\/utils\/resolve-rewrites\.js/,
],
notTests: [/next\/dist\/server\/next\.js/, /next\/dist\/bin/],
notTests: [
/next\/dist\/server\/next\.js/,
/next\/dist\/bin/,
/\0/,
/\?/,
/!/,
],
},
]

Expand All @@ -206,14 +212,27 @@ describe.skip('Production Usage with swcMinify', () => {
)
const { version, files } = JSON.parse(contents)
expect(version).toBe(1)
expect([...new Set(files)].length).toBe(files.length)

expect(
check.tests.every((item) => files.some((file) => item.test(file)))
check.tests.every((item) => {
if (files.some((file) => item.test(file))) {
return true
}
console.error(`Failed to find ${item} in`, files)
return false
})
).toBe(true)

if (sep === '/') {
expect(
check.notTests.some((item) => files.some((file) => item.test(file)))
check.notTests.some((item) => {
if (files.some((file) => item.test(file))) {
console.error(`Found unexpected ${item} in`, files)
return true
}
return false
})
).toBe(false)
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/integration/production/pages/index.js
@@ -1,4 +1,4 @@
import Link from 'next/link'
import Link from 'next/link?loaderQuery'

if (typeof window === 'undefined') {
try {
Expand Down

0 comments on commit 2a5c21b

Please sign in to comment.