fix: support optional params in Netlify split mode - #7
anurag6569201 wants to merge 1 commit into
Conversation
Source PR: sveltejs#17020 Source head: 1a9f6b8
⛔ Shipwright · BlockedRecommendation: do not merge PR #7 · Tier
Findings (3)
Fireworks usage: 7,879 input · 546 output · 8,425 total tokens · $0.0021 · 12s · 0 fix iteration(s) Open the Shipwright check for full evidence and the audit bundle. Use |
| const pattern = `/${parts.join('/')}`; | ||
| const name = | ||
| FUNCTION_PREFIX + (parts.join('-').replace(/[:.]/g, '_').replace('*', '__rest') || 'index'); | ||
| FUNCTION_PREFIX + |
There was a problem hiding this comment.
Shipwright · CRITICAL
The function name generation uses 'parts.join('-')' where 'parts' now contains ':param1?'.
Impact: The function name generation uses 'parts.join('-')' where 'parts' now contains ':param1?'. The '.replace(/[:.]/g, '_')' runs before '.replace(/?/g, '')', so the '?' is removed, but the test expects the file 'sveltekit-collection-_param1-article.mjs'. However, for a route with multiple optional params or a rest segment combined with optional params, the '?' removal can create name collisions. More critically, the 's…
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
|
|
||
| const patterns = [pattern, `${pattern === '/' ? '' : pattern}/__data.json`]; | ||
| patterns.forEach((p) => seen.add(p)); | ||
| patterns.forEach((pattern) => seen.add(pattern)); |
There was a problem hiding this comment.
Shipwright · HIGH
The variable shadowing in 'patterns.forEach((pattern) => seen.add(pattern))' reuses the name 'pattern' for the loop parameter, shadowing the outer 'const pattern'.
Impact: The variable shadowing in 'patterns.forEach((pattern) => seen.add(pattern))' reuses the name 'pattern' for the loop parameter, shadowing the outer 'const pattern'. This is confusing and error-prone; a future maintainer modifying the loop body may inadvertently reference the wrong 'pattern'. The original code used 'p' to avoid this. This change reduces clarity without functional benefit.
Suggested fix: Review the cited evidence, fix the risk if confirmed, and rerun Shipwright.
closes sveltejs#9825
This maps
[[optional]]segments to URLPattern's native optional named-parameter syntax (:param?).Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits
Summary by CodeRabbit
Bug Fixes
Tests
Source merge-base:
3b688deb803e77ab39c2c7adf81c5c3c168f73d7Source head:
1a9f6b8b3d34e9eb97d49c783e82b9d1fc057245