perf(router-core): guard default search parser against throws - #7665
Closed
anonrig wants to merge 1 commit into
Closed
perf(router-core): guard default search parser against throws#7665anonrig wants to merge 1 commit into
anonrig wants to merge 1 commit into
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
Stop the default search parser from throwing a
SyntaxErrorfor every plain-string param. Inpackages/router-core/src/searchParams.ts,defaultParseSearchranJSON.parseon every leftover string value:parseMaybeJsonfirst checks, from the first non-whitespace character, whether the value could be JSON and only then callsJSON.parse. The check is a superset of JSON's value-start grammar ({ [ " -0-9t f n, skipping leading whitespace), so any stringJSON.parsewould accept still gets parsed — we only skip the doomed parse of plain strings.Why
qss.decodealready coerces numbers/booleans, soJSON.parseonly ever runs on the remaining string values — which on real apps are mostly plain text (?q=hello&f=live&src=typed_query). Each one throws aSyntaxErrorthatparseSearchWithcatches to keep the raw string, and constructing + throwing that error (with stack capture) is the dominant cost.Search params are parsed on every SSR request and every client navigation. In-situ before/after on the real
defaultParseSearch(Vitestbench):JSON.parse)?q=…&f=live&src=…&pf=1)?q=from:elon since:2024&f=live&…)Correctness
Behavior is identical. The guard never skips a value
JSON.parsewould accept, and for skipped values it returns the raw string — exactly whatparseSearchWithdoes today whenJSON.parsethrows.Verified by:
searchParamssuite (isomorphism round-trips + "alien deserialization" of human-typed params).couldBeJsonis a correct superset ofJSON.parse's accept set.t/f/n-prefixed non-literals (tweet,false_alarm,null_island) stay strings.Notes
defaultStringifySearchuses the sameJSON.parse-as-throw-probe pattern when building URLs (also hot via<Link>), but its logic relies on throw-vs-no-throw as the "is this JSON?" signal, so making it allocation-free cleanly is a slightly larger change — happy to do it as a follow-up.@tanstack/router-corepatch).unused-imports/no-unused-varswarnings on the existingcatch (_err)blocks are untouched by this change.Summary by CodeRabbit