feat(admin-vite-plugin,dashboard): support for react-router's splat and optional segments #13547
Conversation
🦋 Changeset detectedLatest commit: d92105b The changes in this PR will be included in the next version bump. This PR includes changesets to release 71 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
@leobenzol is attempting to deploy a commit to the medusajs Team on Vercel. A member of the Team first needs to authorize it. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| const leaf = createLeafRoute(Component, loader) | ||
| leaf.children = processParallelRoutes(parallelRoutes, currentFullPath) | ||
| route.children.push(leaf) | ||
| } |
There was a problem hiding this comment.
Bug: Splat Route Configuration Overwrite Bug
For splat routes (* or *?), Object.assign(route, leaf) incorrectly overwrites the route's path, handle, loader, and children. This causes the route to lose its parent path and configuration, and can lead to errors if children becomes undefined. The code also processes remaining segments after a splat route, which violates React Router's terminal splat segment rule, creating invalid route structures.
|
@adrien2p @carlos-r-l-rodrigues @fPolic @olivermrbl would like you opinion on this :) |
|
@leobenzol the team is busy at the moment, I'll get a member to review this one asap |
|
@willbouch don't worry I know yall are busy for 2.10.4, I have no problem waiting until after the release 🙌 |
2 participants
Closes #12348
This PR adds support for the missing route segment types available in react-router, namely splat(called catch-all in next.js) routes and optional static/dynamic/splat routes.
Added translation from folder name to react-router symbol with the 4 .replace() lines
Most people will only ever use optional splat and dynamic but I saw no reason to exclude the other two. I chose
[[*]], [*], [[id]], (static)to somewhat align with next.js and the existing [id]. I used*instead of...since that's what useParams() returns.Just my initial take, I can change the naming if needed or remove
[*]and(static)if it's too much(they are somewhat useless after all)Allow routes with an optional segment at the end to be displayed in the sidebar menu items
Fixed edge-case for splat routes in the logic for generating the react-router map, explained in comment