remix
remix copied to clipboard
fix: check index routes against route config instead of id suffix
Index routes in Remix Flat Routes do not end in /index
so these routes were not correctly identified.
⚠️ No Changeset found
Latest commit: 17581e50c110fa752442f59ffd5facf18048cd6e
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
@kentcdodds Yes, I'll write a test for this. I was just trying to at least get this on the radar.
In the meantime, I think I'll just update Remix Flat Routes to add the /index
to the id for index routes. That way it's a non-blocker until this PR gets merged.
Ok, v0.4.8 has been published. This updates the route ids for index routes to match what Remix is looking for. At least until Remix merges my PR.
@kentcdodds BTW, I just realized I don't need a specific test for this, because ALL the existing tests pass. That means I didn't break anything. Just changed how the index route is determined.
The benefit is to:
- Demonstrate the bug
- Make sure it never happens again
Hi. I opened a PR back in May 7 that fixed this: https://github.com/remix-run/remix/pull/3127.
@penspinner Nice.. I didn't see that one. Cool that you have tests.
BTW: it looks like there is one other place where it incorrectly checks for /index:
https://github.com/remix-run/remix/pull/4560/files?diff=unified&w=1#diff-67cd998c708c10f278fa0382338e65a4d43e3fd67e4ed5bc984d13d1d1f9725f
@kiliman, thanks. Updated.
I'm going to close this PR in favor of #3127.