Address review feedback: extract modules, clean up imports#90813
Merged
lukesandberg merged 10 commits intocanaryfrom Mar 4, 2026
Merged
Address review feedback: extract modules, clean up imports#90813lukesandberg merged 10 commits intocanaryfrom
lukesandberg merged 10 commits intocanaryfrom
Conversation
Collaborator
Tests Passed |
Collaborator
Stats from current PR✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
📦 Dev Server (Webpack) (Legacy)📦 Dev Server (Webpack)
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles: **401 kB** → **401 kB** ✅ -27 B80 files with content-based hashes (individual files not comparable between builds) Server Middleware
Build DetailsBuild Manifests
📦 WebpackClient Main Bundles
Polyfills
Pages
Server Edge SSR
Middleware
Build DetailsBuild Manifests
Build Cache
🔄 Shared (bundler-independent)Runtimes
📎 Tarball URL |
70ecbad to
19997ca
Compare
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…ects swc/options.ts imports these helpers during Jest config setup. When it imported from utils.ts (before the webpack-layer extraction), it would trigger the side-effect imports of node-environment modules. This was required for certain tests (like fast-set-immediate.external.test.ts) to work correctly. By changing swc/options.ts to import from utils.ts (which re-exports from webpack-layer.ts), we maintain the side-effect loading while keeping the webpack-layer.ts extraction intact.
…side effects" This reverts commit 8866943.
…mpatibility
Jest runs tests in a VM context where the process object has a separate
EventEmitter from the real Node.js process. This causes process.on('unhandledRejection')
handlers added in tests to not receive events that fire on the real process.
The unhandled-rejection.external filter patches process.on() to redirect
'unhandledRejection' listeners into an internal queue, bypassing EventEmitter's
this-based storage. This allows both the host and VM contexts to share handlers.
By loading this filter in globalSetup (which runs in the host context), tests
like fast-set-immediate.external.test.ts can properly intercept unhandled
rejections for verification.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The webpack-layer extraction broke the transitive dependency chain that pulled async-sema into the server trace via utils.ts.
a8163d4 to
e4c9d54
Compare
lukesandberg
approved these changes
Mar 4, 2026
sokra
added a commit
that referenced
this pull request
Mar 6, 2026
## Summary Follow-up to #90514 addressing review feedback from @lukesandberg: - **Extract webpack layer helpers to `webpack-layer.ts`**: Moves `shouldUseReactServerCondition` and `isWebpackAppPagesLayer` out of the large `utils.ts` into their own module, reducing transitive dependencies pulled into `swc/options.ts` (reverts the revert from the previous PR) - **Restore top-level import of `throwTurbopackInternalError`**: The lazy `require()` in `swc/index.ts` was unnecessary since `createProject` is always called — restores the simpler top-level import - **Remove dead `getSupportedBrowsers` re-export from `utils.ts`**: All importers already import directly from `get-supported-browsers.ts`, so the re-export was unused - **Extract `formatIssue` and `isRelevantWarning` to `format-issue.ts`**: Moves these functions (and their private helpers) out of the large `turbopack/utils.ts` into a dedicated module, so `print-build-errors.ts` doesn't pull in all of `turbopack/utils` ## Test plan - `pnpm --filter=next types` passes - Unit tests for `formatIssue` pass (`utils.test.ts` updated to import from new module) - NFT test expectations updated for the changed module graph
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Follow-up to #90514 addressing review feedback from @lukesandberg:
webpack-layer.ts: MovesshouldUseReactServerConditionandisWebpackAppPagesLayerout of the largeutils.tsinto their own module, reducing transitive dependencies pulled intoswc/options.ts(reverts the revert from the previous PR)throwTurbopackInternalError: The lazyrequire()inswc/index.tswas unnecessary sincecreateProjectis always called — restores the simpler top-level importgetSupportedBrowsersre-export fromutils.ts: All importers already import directly fromget-supported-browsers.ts, so the re-export was unusedformatIssueandisRelevantWarningtoformat-issue.ts: Moves these functions (and their private helpers) out of the largeturbopack/utils.tsinto a dedicated module, soprint-build-errors.tsdoesn't pull in all ofturbopack/utilsTest plan
pnpm --filter=next typespassesformatIssuepass (utils.test.tsupdated to import from new module)