Turbopack: Align some "Module not found" errors with webpack #58518
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This: - Sends an hmr sync event so that errors that occur after the initial hmr connection are sent to the client - Aligns on `path/to/file.js:line:column` format across error overlay implementations in the cli and on the web - Adapts "Module not found" errors from Turbopack to include Next.js-relevant formatting and documentation links to align with webpack Test Plan: Passes 3 tests that were previously failing
wbinnssmith
requested review from
timneutkens,
ijjk,
shuding,
huozhi,
a team,
feedthejim,
ztanner and
wyattjoh
as code owners
ijjk
added
area: tests
created-by: Turbopack team
Tests Passed |
Stats from current PRDefault BuildGeneral Overall increase
|
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| buildDuration | 10.4s | 10.5s | |
| buildDurationCached | 6s | 6.1s | N/A |
| nodeModulesSize | 199 MB | 199 MB | |
| nextStartRea..uration (ms) | 421ms | 421ms | ✓ |
Client Bundles (main, webpack)
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| 199-HASH.js gzip | 30.7 kB | 30.7 kB | N/A |
| 3f784ff6-HASH.js gzip | 53.3 kB | 53.3 kB | ✓ |
| 494.HASH.js gzip | 180 B | 181 B | N/A |
| framework-HASH.js gzip | 45.2 kB | 45.2 kB | ✓ |
| main-app-HASH.js gzip | 241 B | 240 B | N/A |
| main-HASH.js gzip | 31.7 kB | 31.7 kB | N/A |
| webpack-HASH.js gzip | 1.7 kB | 1.7 kB | ✓ |
| Overall change | 100 kB | 100 kB | ✓ |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| _app-HASH.js gzip | 194 B | 195 B | N/A |
| _error-HASH.js gzip | 182 B | 181 B | N/A |
| amp-HASH.js gzip | 501 B | 503 B | N/A |
| css-HASH.js gzip | 322 B | 323 B | N/A |
| dynamic-HASH.js gzip | 2.5 kB | 2.5 kB | ✓ |
| edge-ssr-HASH.js gzip | 253 B | 255 B | N/A |
| head-HASH.js gzip | 348 B | 347 B | N/A |
| hooks-HASH.js gzip | 369 B | 368 B | N/A |
| image-HASH.js gzip | 4.27 kB | 4.27 kB | N/A |
| index-HASH.js gzip | 256 B | 256 B | ✓ |
| link-HASH.js gzip | 2.61 kB | 2.6 kB | N/A |
| routerDirect..HASH.js gzip | 311 B | 311 B | ✓ |
| script-HASH.js gzip | 384 B | 383 B | N/A |
| withRouter-HASH.js gzip | 307 B | 308 B | N/A |
| 1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
| Overall change | 3.17 kB | 3.17 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| _buildManifest.js gzip | 484 B | 483 B | N/A |
| Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| index.html gzip | 528 B | 526 B | N/A |
| link.html gzip | 538 B | 541 B | N/A |
| withRouter.html gzip | 524 B | 523 B | N/A |
| Overall change | 0 B | 0 B | ✓ |
Edge SSR bundle Size
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| edge-ssr.js gzip | 92.6 kB | 92.6 kB | N/A |
| page.js gzip | 146 kB | 146 kB | N/A |
| Overall change | 0 B | 0 B | ✓ |
Middleware size
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| middleware-b..fest.js gzip | 626 B | 627 B | N/A |
| middleware-r..fest.js gzip | 150 B | 151 B | N/A |
| middleware.js gzip | 35.7 kB | 35.7 kB | ✓ |
| edge-runtime..pack.js gzip | 1.92 kB | 1.92 kB | ✓ |
| Overall change | 37.6 kB | 37.6 kB | ✓ |
Next Runtimes
| vercel/next.js canary | vercel/next.js wbinnssmith/module-not-found | Change | |
|---|---|---|---|
| app-page-exp...dev.js gzip | 168 kB | 168 kB | ✓ |
| app-page-exp..prod.js gzip | 93.7 kB | 93.7 kB | ✓ |
| app-page-tur..prod.js gzip | 94.5 kB | 94.5 kB | ✓ |
| app-page-tur..prod.js gzip | 89 kB | 89 kB | ✓ |
| app-page.run...dev.js gzip | 138 kB | 138 kB | ✓ |
| app-page.run..prod.js gzip | 88.4 kB | 88.4 kB | ✓ |
| app-route-ex...dev.js gzip | 24.2 kB | 24.2 kB | ✓ |
| app-route-ex..prod.js gzip | 16.8 kB | 16.8 kB | ✓ |
| app-route-tu..prod.js gzip | 16.9 kB | 16.9 kB | ✓ |
| app-route-tu..prod.js gzip | 16.4 kB | 16.4 kB | ✓ |
| app-route.ru...dev.js gzip | 23.6 kB | 23.6 kB | ✓ |
| app-route.ru..prod.js gzip | 16.4 kB | 16.4 kB | ✓ |
| pages-api-tu..prod.js gzip | 9.37 kB | 9.37 kB | ✓ |
| pages-api.ru...dev.js gzip | 9.64 kB | 9.64 kB | ✓ |
| pages-api.ru..prod.js gzip | 9.37 kB | 9.37 kB | ✓ |
| pages-turbo...prod.js gzip | 21.9 kB | 21.9 kB | ✓ |
| pages.runtim...dev.js gzip | 22.6 kB | 22.6 kB | ✓ |
| pages.runtim..prod.js gzip | 21.9 kB | 21.9 kB | ✓ |
| server.runti..prod.js gzip | 49.3 kB | 49.3 kB | ✓ |
| Overall change | 930 kB | 930 kB | ✓ |
sokra
reviewed
Nov 17, 2023
| } | ||
| } else { | ||
| message = `${formattedTitle}` | ||
| message = title.replace(/\n/g, '\n ') |
There was a problem hiding this comment.
It's a bit weird that we only show the title or the error when there is not source. I guess the title should always be added to the message.
sokra
reviewed
Nov 17, 2023
| @@ -1304,6 +1315,26 @@ async function startWatcher(opts: SetupOpts) { | |||
| type: HMR_ACTIONS_SENT_TO_BROWSER.TURBOPACK_CONNECTED, | |||
| } | |||
| client.send(JSON.stringify(turbopackConnected)) | |||
|
|
|||
| const errors = [] | |||
| for (const pageIssues of issues.values()) { | |||
There was a problem hiding this comment.
I feel like we should only send errors for the current page. Maybe we can send them on the first ping event instead. There we would have access to the page.
There was a problem hiding this comment.
The ping event comes too late -- we need to have the issues at the time the error overlay renders.
There was a problem hiding this comment.
Could we put a header on the WebSocket request that tells the current page and update the issues on ping if the page changes?
…ebpack" This reverts commit 2edee1e.
sokra
approved these changes
Nov 30, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This:
path/to/file.js:line:columnformat across error overlay implementations in the cli and on the webTest Plan: Passes 3 tests that were previously failing
Closes PACK-1974