refactor(app-browser-entry): dedupe encoded JSON header parsing#1070
refactor(app-browser-entry): dedupe encoded JSON header parsing#1070james-elicx merged 1 commit intomainfrom
Conversation
Extract the repeated try/catch JSON.parse(decodeURIComponent(...)) block into a single helper. Follow-up to #1049. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
commit: |
|
|
/bigbonk review |
There was a problem hiding this comment.
Clean refactor. The helper is well-scoped, the two call sites read better, and the behavioral nuance (separating applyClientParams from the parse try/catch at the first call site) is handled correctly.
Two minor observations, neither blocking:
-
!valueguard treats""as missing —parseEncodedJsonHeader("")returnsnullbecause the empty string is falsy. That's fine here sinceheaders.get()returnsnullfor absent headers and an emptyX-Vinext-Paramsvalue would failJSON.parseanyway, so the short-circuit is just saving a wasted decode+parse. Worth noting in case the helper is ever promoted to a shared utility where an empty string might carry different semantics. -
The generic
as Tcast is unchecked at runtime — the caller trusts that the JSON payload matchesRecord<string, string | string[]>. That's inherited from the original code and not a regression, just a note that if this helper ever gets reused for other header shapes, a runtime schema check (or at least a comment about the trust boundary) would be warranted.
LGTM — all CI green, behavior preserved, code is cleaner.
|
Review posted and PR approved. The refactor is straightforward and correct:
|
Summary
try { JSON.parse(decodeURIComponent(headerValue)) } catch { ... }pattern used to hydrate params from theX-Vinext-Paramsheader into a single local helper,parseEncodedJsonHeader<T>(value: string | null): T | null.packages/vinext/src/server/app-browser-entry.ts(the initial RSC hydration path and the in-flight navigation path) now share the helper.Follow-up to #1049.
Files changed
packages/vinext/src/server/app-browser-entry.ts— adds the helper at the top of the file and updates the two call sites.Notes on behavior preservation
The two original blocks had a subtle difference:
applyClientParams(params)inside the sametry, so the catch silently swallowed any error fromapplyClientParamsas well.The helper itself only handles parsing. To preserve the original error-swallowing semantics at the first call site,
applyClientParamsis wrapped in a small dedicatedtry/catchthere. The helper does not log or surface errors — silent by design, matching the pre-existing behavior.A quick
grep -rn "JSON.parse(decodeURIComponent" packages/vinext/src/confirmed the pattern only appears in this file, so a module-private helper (rather than a shared utility) is sufficient.Test plan
pnpm vp test run tests/app-router.test.ts— 308 tests passpnpm fmt --writeon the touched file🤖 Generated with Claude Code