fix(react-router): support React Router 6 style relative paths in IonRouterOutlet #30844
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
force-pushed
the
fix/react-router-6-paths
branch
from
f31ff3e to
5c66ea0
Compare
|
I think there is a minor documentation issue here: |
|
I just installed the test branch for this: Unfortunately, this immediately breaks my app. I have all my Changing all |
|
@ptmkenny Can you provide a reproduction of this? I can't see how this change would cause that and I can't see how the e2e tests would be passing (much less building) if this were happening. For example, this newly added one in this PR, which uses |
|
@ShaneK Sorry about that, it seems there was some weird caching issue that caused that error. I rebooted the machine, blew away all the caches, and I could build my app again with the new build. However, it seems that not having a slash in a root-level route will still break the app. See the repro here: https://github.com/ptmkenny/ionic-react-router-6-test/tree/router-outlet I put the reproduction instructions in the README.md. Also, note that in addition to adding the slash (for IonRouterOutlet), you can also replace IonRouterOutlet with Routes (without adding the slash) and the page will be shown. |
…work into fix/react-router-6-paths
|
@ptmkenny Wow, that was a big one! I don't know how I didn't have that as a test case before, but I do now. I have a dev build up that will hopefully work for you now, as well as having this case as part of my E2E tests: |
|
@ShaneK Thank you for the speedy fix, but it seems to be a little more complicated. Follow-up: #24177 (comment) |
|
That would be a new issue, not the same one. You should report it in the main thread and I'll deal with it in another PR |
packages/react-router/test/base/src/pages/relative-paths/RelativePaths.tsx
Show resolved
Hide resolved
packages/react-router/test/base/src/pages/relative-paths/RelativePaths.tsx
Outdated
Show resolved
Hide resolved
| cy.get('[data-testid="page-b-content"]').should('contain', 'Page B'); | ||
| }); | ||
|
|
||
| it('should navigate to Page C (defined with relative path - no leading slash)', () => { |
There was a problem hiding this comment.
What's the point of a 2nd test for a relative path?
There was a problem hiding this comment.
I thought I was testing another edge case, but it seems not. Probably something I ended up forgetting to clean up. Good catch! f570837
ShaneK
force-pushed
the
sk/react-router-6
branch
2 times, most recently
from
0c82ee2 to
6a61ecf
Compare
…work into fix/react-router-6-paths
…c-framework into fix/react-router-6-paths
Issue number: resolves an issue from a comment
What is the current behavior?
Currently, Ionic's RR6 doesn't support relative routes in the same way RR6 does. Routes that do not start with a
/do not work in the Ionic RR6 implementation in some cases.What is the new behavior?
With this change, we properly support these route styles and more closely align with normal RR6 route support.
Does this introduce a breaking change?
Other information
Current dev build:
This PR will be merged into the RR6 branch, which will be squash+merged into the major 9 branch. This will prevent major 9 from having commits in the change log on release that reference fixing things that are only available in major 9.