Refactor extractExportedConstValue to return { value } | null instead of throwing#90510
Open
Refactor extractExportedConstValue to return { value } | null instead of throwing#90510
Conversation
nextjs-bot
added
created-by: Turbopack team
Collaborator
Failing test suitesCommit: 9f9107f | About building and testing Next.js
Expand output● Handle url imports › should render the /static page ● Handle url imports › should client-render the /static page ● Handle url imports › should render the /ssr page ● Handle url imports › should client-render the /ssr page ● Handle url imports › should render the /ssg page ● Handle url imports › should client-render the /ssg page ● Handle url imports › should render a static url image import ● Handle url imports › should allow url import in css ● Handle url imports › should respond on value api
Expand output● emitDecoratorMetadata SWC option › should compile with emitDecoratorMetadata enabled ● emitDecoratorMetadata SWC option › should compile with emitDecoratorMetadata enabled for API ● Test suite failed to run
Expand output● Telemetry CLI › production mode › cli session: babel tooling config ● Telemetry CLI › production mode › cli session: custom babel config (plugin) ● Telemetry CLI › production mode › cli session: package.json custom babel config (plugin) ● Telemetry CLI › production mode › cli session: custom babel config (preset) ● Telemetry CLI › production mode › cli session: next config with webpack ● Telemetry CLI › production mode › detect static 404 correctly for ● Telemetry CLI › production mode › detect page counts correctly for |
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: **400 kB** → **400 kB** ✅ -13 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 |
… 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
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
2 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)