Skip to content

refactor(router): drop a type check and a field reset that change nothing - #200

Merged
dinwwwh merged 1 commit into
mainfrom
claude/project-thread-m32ra9
Oct 29, 2026
Merged

dinwwwh merged 1 commit into
mainfrom
claude/project-thread-m32ra9

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 29, 2026

Copy link
Copy Markdown
Member

Requested by dinwwwh · project thread

What

Two pieces of code that never change a result are removed.

  • addParsed, for a path with % but no {: the check no longer tests whether each segment is a string. toSegment returns a param only for text that starts with {, so such a path parses into strings only. The segments are now typed string[], with a comment saying why. Stryker had reported the typeof test as a surviving mutant for this reason.
  • literalChild, when a node's second literal child turns its children into a map: literalNode is no longer set back to undefined. find reads literalNode only while literal is set, and literalChild reads it only while literals is unset, so nothing reads the old value. Its doc comment now says it holds the only child "while literal holds its text".

I also tried keeping literal set as well and having find check the map first. That cut 10 B gzip but cost 0.4–1% per request with params, so it is not in this PR.

Measurements

Instruction counts from callgrind, node --single-threaded, averaged over hash seeds 1 and 2:

before after
min / min+gzip 5053 / 2098 B 5012 / 2092 B
import 2.165M 2.159M
REST cold+3 match 3.504M 3.498M

Steady-state matching is unchanged: find is not touched, and neither changed line runs per request.

Checks

  • Tests pass with 100% coverage.
  • The model fuzzers find 0 diffs.
  • Stryker finds 67 survivors, down from 68. The typeof mutant is gone.

Generated by Claude Code

…hing

A path without `{` parses into literal segments only, so the `%`-path
check in `addParsed` need not test each segment's type. `literalChild`
no longer clears `literalNode` when it moves to a map, as nothing reads
it once `literal` is cleared.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VVaw7Sutbw6xMTMyDFeNfw
@dinwwwh dinwwwh self-assigned this Oct 29, 2026
@codecov

codecov Bot commented Oct 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

1 warning

⚠️ 1 benchmark is dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

✅ 29 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing claude/project-thread-m32ra9 (d5cc526) with main (55cb4e1)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • addParsed literal fast path — dropped the typeof segment === 'string' guard and typed the parsed result string[]. toSegment only yields a RouteParam for text opening with {, and add routes only {-free paths into this branch, so every segment is genuinely a string. The cast is sound and the new comment states the reason.
  • literalChild promotion — removed the node.literalNode = undefined reset when the single literal becomes a map. literalNode is read only in the literals === undefined branch of literalChild and in find only while node.literal !== undefined; the map transition clears literal and literals is never reset to undefined, so the stale value is unreachable. The literalNode doc comment now correctly scopes the field.

Both changes are behavior-neutral; matching (find) is untouched and neither line runs per request.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@dinwwwh
dinwwwh merged commit f18cd06 into main Oct 29, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants