[PR #1040] [MERGED] fix(routing): match inherited slot params against routePath, not request.url #1041

New issue

Closed

opened 2026-05-06 13:11:42 +02:00 by BreizhHardware · 0 comments

BreizhHardware commented

2026-05-06 13:11:42 +02:00

Owner

📋 Pull Request Information

Original PR: https://github.com/cloudflare/vinext/pull/1040
Author: @james-elicx
Created: 5/3/2026
Status: ✅ Merged
Merged: 5/3/2026
Merged by: @james-elicx

Base: main ← Head: fix/inherited-slot-basepath-and-memoize

📝 Commits (1)

f35c27b fix(routing): match inherited slot params against routePath, not request.url

📊 Changes

3 files changed (+99 additions, -15 deletions)

View changed files

📝 packages/vinext/src/routing/app-route-graph.ts (+29 -9)
📝 packages/vinext/src/server/app-page-element-builder.ts (+10 -6)
📝 tests/app-page-element-builder.test.ts (+60 -0)

📄 Description

Follow-up to #1039, addressing two items I missed during review.

Summary

Bug under basePath: buildSlotOverrides was deriving the URL parts to match against slot.slotPatternParts from request.url. The original request URL still carries the configured basePath, but slotPatternParts is built from app-relative segments only — so for any basePath-configured app, the pattern match silently failed and slot params fell back to the route's matched params (the exact bug #1039 was supposed to fix). Switched to the already-normalized routePath parameter (basePath stripped, RSC suffix removed) that buildPageElements already receives.
Memoize findSlotSubPages: cached per-build via a WeakMap keyed by the matcher. Inherited slots get scanned once per descendant route, so without memoization a route N segments deep paid O(N) full subtree walks for every shared ancestor slot. Cache lifetime tracks the matcher (one per build), so a long-lived dev server doesn't accumulate state across builds.
Dedupe: findMirroredSlotPage was calling convertSegmentsToRouteParts(segmentsBelow) twice when the literal-tier match missed. Hoisted it out so both tiers reuse the conversion.

Test plan

New unit test in tests/app-page-element-builder.test.ts exercises the basePath regression: request URL is http://localhost/base/distinct/alice while routePath is /distinct/alice, slot has slotPatternParts: ["distinct", ":name"]. Without the fix, urlParts would be ["base","distinct","alice"], the match would fail, and the slot's resolved params would silently lose the name value. With the fix, slotParams resolves to { name: "alice" }.
pnpm exec vp test run --project unit — 3524 tests pass.
pnpm check — only pre-existing failure (benchmarks/vinext/vite.config.ts cannot resolve vinext until the package is built; same on main).

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/cloudflare/vinext/pull/1040 **Author:** [@james-elicx](https://github.com/james-elicx) **Created:** 5/3/2026 **Status:** ✅ Merged **Merged:** 5/3/2026 **Merged by:** [@james-elicx](https://github.com/james-elicx) **Base:** `main` ← **Head:** `fix/inherited-slot-basepath-and-memoize` --- ### 📝 Commits (1) - [`f35c27b`](https://github.com/cloudflare/vinext/commit/f35c27b66a4e4ff7df01c3a85f68bd3918618e5a) fix(routing): match inherited slot params against routePath, not request.url ### 📊 Changes **3 files changed** (+99 additions, -15 deletions) <details> <summary>View changed files</summary> 📝 `packages/vinext/src/routing/app-route-graph.ts` (+29 -9) 📝 `packages/vinext/src/server/app-page-element-builder.ts` (+10 -6) 📝 `tests/app-page-element-builder.test.ts` (+60 -0) </details> ### 📄 Description Follow-up to [#1039](https://github.com/cloudflare/vinext/pull/1039), addressing two items I missed during review. ## Summary - **Bug under `basePath`**: `buildSlotOverrides` was deriving the URL parts to match against `slot.slotPatternParts` from `request.url`. The original request URL still carries the configured `basePath`, but `slotPatternParts` is built from app-relative segments only — so for any basePath-configured app, the pattern match silently failed and slot params fell back to the route's matched params (the exact bug #1039 was supposed to fix). Switched to the already-normalized `routePath` parameter (basePath stripped, RSC suffix removed) that `buildPageElements` already receives. - **Memoize `findSlotSubPages`**: cached per-build via a `WeakMap` keyed by the matcher. Inherited slots get scanned once per descendant route, so without memoization a route N segments deep paid O(N) full subtree walks for every shared ancestor slot. Cache lifetime tracks the matcher (one per build), so a long-lived dev server doesn't accumulate state across builds. - **Dedupe**: `findMirroredSlotPage` was calling `convertSegmentsToRouteParts(segmentsBelow)` twice when the literal-tier match missed. Hoisted it out so both tiers reuse the conversion. ## Test plan - [x] New unit test in `tests/app-page-element-builder.test.ts` exercises the basePath regression: request URL is `http://localhost/base/distinct/alice` while `routePath` is `/distinct/alice`, slot has `slotPatternParts: ["distinct", ":name"]`. Without the fix, `urlParts` would be `["base","distinct","alice"]`, the match would fail, and the slot's resolved `params` would silently lose the `name` value. With the fix, `slotParams` resolves to `{ name: "alice" }`. - [x] `pnpm exec vp test run --project unit` — 3524 tests pass. - [x] `pnpm check` — only pre-existing failure (`benchmarks/vinext/vite.config.ts` cannot resolve `vinext` until the package is built; same on `main`). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>