Refactor extractExportedConstValue to return { value } | null instead of throwing#90510
Merged
Refactor extractExportedConstValue to return { value } | null instead of throwing#90510
Conversation
nextjs-bot
added
created-by: Turbopack team
Collaborator
Failing test suitesCommit: da277a1 | About building and testing Next.js
Expand output● app dir - basepath › should successfully hard navigate from pages -> app |
Collaborator
Stats from current PR🔴 1 regression, 1 improvement
📊 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** → **400 kB** ✅ -943 B83 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
📝 Changed Files (25 files)Files with changes:
View diffsapp-page-exp..ntime.dev.jsfailed to diffapp-page-exp..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page-tur..ntime.dev.jsfailed to diffapp-page-tur..time.prod.jsfailed to diffapp-page.runtime.dev.jsfailed to diffapp-page.runtime.prod.jsfailed to diffapp-route-ex..ntime.dev.jsDiff too large to display app-route-ex..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route-tu..ntime.dev.jsDiff too large to display app-route-tu..time.prod.jsDiff too large to display app-route.runtime.dev.jsDiff too large to display app-route.ru..time.prod.jsDiff too large to display pages-api-tu..ntime.dev.jsDiff too large to display pages-api-tu..time.prod.jsDiff too large to display pages-api.runtime.dev.jsDiff too large to display pages-api.ru..time.prod.jsDiff too large to display pages-turbo...ntime.dev.jsDiff too large to display pages-turbo...time.prod.jsDiff too large to display pages.runtime.dev.jsDiff too large to display pages.runtime.prod.jsDiff too large to display server.runtime.prod.jsDiff too large to display 📎 Tarball URL |
… of throwing
Replace throw-based control flow with a return-based approach for better
performance. The function now returns `{ value: any } | null` — `null` when
the declaration is not found, `{ value }` when found. UnsupportedValueError
is still thrown for genuinely unsupported values.
Remove NoSuchDeclarationError (no longer needed) and update all callsites
in get-page-static-info.ts to unwrap the result.
…portedValueError
Replace throw-based control flow in extractValue with a discriminated
return type: `{ value: any } | { unsupported: string; path?: string }`.
- Extract path-formatting logic from UnsupportedValueError constructor
into a standalone formatCodePath helper
- All 6 throw sites become return statements
- 3 recursive call sites propagate unsupported results
- Remove UnsupportedValueError class entirely
- Remove try/catch blocks from all 4 callsites in get-page-static-info.ts
- Update warnAboutUnsupportedValue to accept the result type directly
sokra
force-pushed
the
sokra/avoid-throw
branch
from
6e366ed to
9f9107f
Compare
parseModule returns null when SWC parsing fails. After the refactor that removed try-catch from get-page-static-info.ts, extractExportedConstValue could crash on null ast. Accept Module | null and return null early.
mischnic
approved these changes
Mar 3, 2026
Member
Author
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
Refactors
extractExportedConstValueand the internalextractValuefunction to use return values instead of exception-based control flow, improving performance during build analysis.Commit 1: Refactor
extractExportedConstValue{ value: any } | nullinstead of throwingNoSuchDeclarationErrornullwhen the exported const declaration is not found,{ value }when foundNoSuchDeclarationErrorclassCommit 2: Refactor
extractValueExtractValueResult({ value: any } | { unsupported: string; path?: string }) instead of throwingUnsupportedValueErrorUnsupportedValueErrorconstructor into a standaloneformatCodePathhelperUnsupportedValueErrorclass entirelyget-page-static-info.tswarnAboutUnsupportedValueto accept the result type directlyMotivation
Both
extractExportedConstValueandextractValueused throw/catch for expected control flow — a missing declaration and unsupported AST node types are normal cases during build analysis, not exceptional errors. Every page/route processed during build hits these functions multiple times. ConstructingErrorobjects (which capture stack traces) and unwinding the stack is expensive in V8. Returning discriminated result types avoids this overhead entirely.Test Plan
test/unit/parse-page-static-info.test.ts— 5/5)