Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(router): allow empty route.path for correct catchAll ranking #582

Merged
merged 3 commits into from
Jun 6, 2024

Conversation

evenchange4
Copy link
Contributor

@evenchange4 evenchange4 commented Jun 3, 2024

Let's say you have a directory structure like this:

└── src
    └── routes
        ├── $
        │   ├── layout.js
        │   └── page.js
        ├── $symbol
	        ├── calc
	            ├── layout.js
	            └── page.js
        ├── home
        │   ├── layout.js
        │   └── page.js
        └── layout.js

Current

❌ Visit localhost:3000/symbol/calc, the src/routes/$/page will be rendered.

Expect

✅ Visit localhost:3000/symbol/calc, the src/routes/$symbol/calc/page should be exactly matched and rendered.

Root Cause

The rank of pathParser is not correct.

@evenchange4 evenchange4 changed the title fix(router): catch all routes ends with a slash should work [WIP] fix(router): catch all routes ends with a slash should work Jun 3, 2024
@evenchange4 evenchange4 changed the title [WIP] fix(router): catch all routes ends with a slash should work fix(router): catch all routes should not end with slash /*/ -> /* Jun 4, 2024
@evenchange4 evenchange4 changed the title fix(router): catch all routes should not end with slash /*/ -> /* fix(router): allow empty route.path for correct catchAll ranking Jun 6, 2024
@evenchange4 evenchange4 requested a review from liximomo June 6, 2024 03:17
@evenchange4 evenchange4 added the bug Something isn't working label Jun 6, 2024
packages/router/src/pathParserRanker.ts Outdated Show resolved Hide resolved
@liximomo liximomo merged commit a106fee into shuvijs:main Jun 6, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants