fix: matchConfigPattern incorrectly matches :param with literal suffix#191
Conversation
Patterns like `/:slug.md` were falling through to the simple segment
matcher, which treated `slug.md` as the entire parameter name and
matched any single-segment path (including `/`). This caused rewrites
like `{ source: "/:slug.md", destination: "/api/markdown/:slug" }` to
rewrite every page request, resulting in 404s.
Add `/:[\w-]+\./.test(pattern)` to the condition that triggers the
regex-based matcher, which already tokenizes `:slug` + `.` + `md`
correctly.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
/bigbonk review this. Check that we are matching NextJS behavior |
commit: |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Good bug fix. The root cause analysis is correct — patterns like /:slug.md fell through to the simple segment matcher, which treated slug.md as the entire param name and matched any single segment regardless of suffix.
The fix is minimal and correct: adding /:[\ w-]+\./.test(pattern) to the regex-branch guard ensures these patterns are handled by the tokenizer/regex path, which already knows how to decompose :slug + . + md into the right regex (^/([^/]+)\.md$).
I verified:
- The tokenizer in
config-matchers.tsandindex.tscorrectly splits/:slug.mdinto tokens:slug,.,md→ regex([^/]+)\.md - The chained-replace approach in
app-dev-server.tsalso works correctly since.replace(/\./g, '\\.')runs first, then:slug→([^/]+), producing the same final regex prod-server.tsimports fromconfig-matchers.ts, so it gets the fix automatically — no separate change needed- The new guard regex has no false positives on patterns without dots (e.g.
/:slug,/path/:slug) - All three copies (
config-matchers.ts,index.ts,app-dev-server.ts) are updated consistently
Two minor observations below, neither blocking.
| pattern.includes("\\") || | ||
| /:[\w-]+[*+][^/]/.test(pattern) | ||
| /:[\w-]+[*+][^/]/.test(pattern) || | ||
| /:[\w-]+\./.test(pattern) |
There was a problem hiding this comment.
Nit: The existing copies of matchConfigPattern in index.ts and app-dev-server.ts are a maintenance risk — this PR is a good example of needing to patch 3 places. Not something to fix in this PR, but worth noting for a future dedup effort.
| "../packages/vinext/src/index.js" | ||
| ); | ||
| // Should match URLs with the .md suffix and extract the param | ||
| expect(matchConfigPattern("/hello-world.md", "/:slug.md")).toEqual({ slug: "hello-world" }); |
There was a problem hiding this comment.
Nice coverage. One edge case that might be worth adding in a follow-up: a pattern with multiple dots like /:file.tar.gz — the tokenizer would produce ([^/]+)\.tar\.gz which is correct, but an explicit test would document that behavior.
|
fatal: could not read Username for 'https://github.com': No such device or address |
|
@southpolesteve Bonk workflow failed. Check the logs for details. View workflow run · To retry, trigger Bonk again. |
2 participants
Summary
/:slug.mdfell through to the simple segment matcher, which treatedslug.mdas the entire parameter name and matched any single-segment path (including/){ source: "/:slug.md", destination: "/api/markdown/:slug" }to rewrite every page request, resulting in 404s on all routes/:[\w-]+\./.test(pattern)to the condition that triggers the regex-based matcher, which already tokenizes:slug+.+mdcorrectlyTest plan
/:slug.mdpattern via bothindex.tsandconfig/config-matchers.tsmatchConfigPatterntests pass (no regressions)GET /returns 200 with HTML (was 404 before)🤖 Generated with Claude Code