-
Notifications
You must be signed in to change notification settings - Fork 26.2k
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
fix(log): improve error when dynamic code eval is disallowed #62999
Conversation
Tests Passed |
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
buildDuration | 13.7s | 13.8s | N/A |
buildDurationCached | 7.4s | 6.7s | N/A |
nodeModulesSize | 198 MB | 198 MB | |
nextStartRea..uration (ms) | 436ms | 436ms | ✓ |
Client Bundles (main, webpack)
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
2453-HASH.js gzip | 30.5 kB | 30.5 kB | N/A |
3304.HASH.js gzip | 181 B | 181 B | ✓ |
3f784ff6-HASH.js gzip | 53.7 kB | 53.7 kB | N/A |
8299-HASH.js gzip | 5.04 kB | 5.04 kB | N/A |
framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
main-app-HASH.js gzip | 242 B | 242 B | ✓ |
main-HASH.js gzip | 32.2 kB | 32.2 kB | N/A |
webpack-HASH.js gzip | 1.68 kB | 1.68 kB | N/A |
Overall change | 45.6 kB | 45.6 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
_app-HASH.js gzip | 196 B | 197 B | N/A |
_error-HASH.js gzip | 184 B | 184 B | ✓ |
amp-HASH.js gzip | 505 B | 505 B | ✓ |
css-HASH.js gzip | 324 B | 325 B | N/A |
dynamic-HASH.js gzip | 2.5 kB | 2.5 kB | N/A |
edge-ssr-HASH.js gzip | 258 B | 258 B | ✓ |
head-HASH.js gzip | 352 B | 352 B | ✓ |
hooks-HASH.js gzip | 370 B | 371 B | N/A |
image-HASH.js gzip | 4.21 kB | 4.21 kB | ✓ |
index-HASH.js gzip | 259 B | 259 B | ✓ |
link-HASH.js gzip | 2.67 kB | 2.67 kB | N/A |
routerDirect..HASH.js gzip | 314 B | 312 B | N/A |
script-HASH.js gzip | 386 B | 386 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 6.57 kB | 6.57 kB | ✓ |
Client Build Manifests
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
_buildManifest.js gzip | 481 B | 484 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
index.html gzip | 530 B | 529 B | N/A |
link.html gzip | 541 B | 541 B | ✓ |
withRouter.html gzip | 524 B | 523 B | N/A |
Overall change | 541 B | 541 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
edge-ssr.js gzip | 95.1 kB | 95.1 kB | N/A |
page.js gzip | 3.07 kB | 3.07 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 625 B | 623 B | N/A |
middleware-r..fest.js gzip | 151 B | 151 B | ✓ |
middleware.js gzip | 25.5 kB | 25.5 kB | N/A |
edge-runtime..pack.js gzip | 839 B | 839 B | ✓ |
Overall change | 990 B | 990 B | ✓ |
Next Runtimes
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 171 kB | 171 kB | ✓ |
app-page-exp..prod.js gzip | 96.5 kB | 96.5 kB | ✓ |
app-page-tur..prod.js gzip | 98.3 kB | 98.3 kB | ✓ |
app-page-tur..prod.js gzip | 92.7 kB | 92.7 kB | ✓ |
app-page.run...dev.js gzip | 149 kB | 149 kB | ✓ |
app-page.run..prod.js gzip | 91.2 kB | 91.2 kB | ✓ |
app-route-ex...dev.js gzip | 21.3 kB | 21.3 kB | ✓ |
app-route-ex..prod.js gzip | 15 kB | 15 kB | ✓ |
app-route-tu..prod.js gzip | 15 kB | 15 kB | ✓ |
app-route-tu..prod.js gzip | 14.8 kB | 14.8 kB | ✓ |
app-route.ru...dev.js gzip | 21 kB | 21 kB | ✓ |
app-route.ru..prod.js gzip | 14.8 kB | 14.8 kB | ✓ |
pages-api-tu..prod.js gzip | 9.52 kB | 9.52 kB | ✓ |
pages-api.ru...dev.js gzip | 9.8 kB | 9.8 kB | ✓ |
pages-api.ru..prod.js gzip | 9.52 kB | 9.52 kB | ✓ |
pages-turbo...prod.js gzip | 22.3 kB | 22.3 kB | ✓ |
pages.runtim...dev.js gzip | 22.9 kB | 22.9 kB | ✓ |
pages.runtim..prod.js gzip | 22.3 kB | 22.3 kB | ✓ |
server.runti..prod.js gzip | 50.5 kB | 50.5 kB | ✓ |
Overall change | 948 kB | 948 kB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | vercel/next.js fix/improve-error-dynamic-eval | Change | |
---|---|---|---|
0.pack gzip | 1.56 MB | 1.56 MB | |
index.pack gzip | 105 kB | 105 kB | |
Overall change | 1.66 MB | 1.66 MB |
Diff details
Diff for middleware.js
Diff too large to display
test/production/app-dir/import-trace-dynamic-eval/import-trace-dynamic-eval.test.ts
Outdated
Show resolved
Hide resolved
...ext/src/build/webpack/plugins/wellknown-errors-plugin/parse-dynamic-code-evaluation-error.ts
Show resolved
Hide resolved
test/production/app-dir/import-trace-dynamic-eval/import-trace-dynamic-eval.test.ts
Outdated
Show resolved
Hide resolved
test/production/app-dir/import-trace-dynamic-eval/next.config.js
Outdated
Show resolved
Hide resolved
test/e2e/app-dir/import-trace-dynamic-eval/node_modules/package.json
Outdated
Show resolved
Hide resolved
…ercel/next.js into fix/improve-error-dynamic-eval
) | ||
const output = await next.build() | ||
expect(output.exitCode).toBe(1) | ||
expect(output.cliOutput.split('\n').slice(3).join('\n')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drops the next.js version, linting and compiling from the output
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use toContain
to check different parts are included:
toContain(Dynamic Code Evaluation ...) // the text
toContain(Import trace for requested module: ...) // the module trace
Inline snapshot might easily change if we changed sth, since we don't have strong requirement to match the format we can just check if the cirtical content are included
|
||
describe('Dynamic Code Evaluation (DCE)', () => { | ||
const { next } = nextTestSetup({ | ||
skipStart: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need to have skipStart
and manually build/start the app. Just need to check the next.cliOutput
in each test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Without skipStart
, building and starting the prod server happens before the tests are even run.
Our naming here is not ideal. skipStart: false
(default) will actually run both build and run: https://github.com/vercel/next.js/blob/canary/test/lib/next-modes/next-start.ts/#L40
so skipStart: true
will stop both of these and is necessary here, so that we can manually do next.build()
in each test run. (we expect different outputs from the CLI in the two test cases)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah agree, could add sth like skipServerStart
or buildOnly
later
) | ||
const output = await next.build() | ||
expect(output.exitCode).toBe(1) | ||
expect(output.cliOutput.split('\n').slice(3).join('\n')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use toContain
to check different parts are included:
toContain(Dynamic Code Evaluation ...) // the text
toContain(Import trace for requested module: ...) // the module trace
Inline snapshot might easily change if we changed sth, since we don't have strong requirement to match the format we can just check if the cirtical content are included
What?
Before:
After:
Why?
Make it easier to track down why the error occurred, with an import trace in the user's code, not node_modules
How?
using
getModuleTrace
, we can track the import trace better.Note: From my experience, we are not catching dynamic eval in Route Handlers (edge), that should probably error. So this PR only addresses Middleware for now. I will create a separate task for Route Handlers.
Closes NEXT-2724