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 leading slash creating incorrect conflict resolution between pages generated from static routes and rest parameters #10607
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: d4bd99e The changes in this PR will be included in the next version bump. 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 |
I just noticed, that calling |
Maybe the Changeset should be updated, to include dynamic-routes too, |
Added second test for dynamic route with leading slash, which I locally ran with and without my fix. |
Changes
Following the PR #10298 to fix issue #9103, my specific problem was still not fixed, as mentioned in #9103 (comment).
In #9103 (comment) I explained my following findings:
//test/ing/
//
with/
frankbits@737d407route.generate()
withstringifyParams()
which callsroute.generate()
internally, after trimming slashes frankbits@78d604cJSON.stringify()
of the return-value ofstringifyParams()
, as the returned value ofroute.generate()
is already a string, and I had to parse it back to a string without""
and the other 2 places where the function is used should not have any problem anywhere without theJSON.stringify()
, as the return-value i used as an array-key to store/retrieve "static path"-objects and only have to fit to each other.Testing
I added frankbits@a913d04 ( edited frankbits@7cba77b, frankbits@22fb67e) a test with the slug having a leading slash (similar to the failing path in my MRE of my original issue #9103)
Also added a test for leading slash in a dynamic route d4bd99e (see #10607 (comment))
All test in
packages/astro/test/dynamic-route-collision.test.js
finish successfully.I couldn't run all test of the project, as
pnpm run test
ran into errors setting up tests.Docs
/cc @withastro/maintainers-docs for feedback!
I have not updated the docs, as my change is not in conflict with the documented behaviour.
But it could probably be sensible to add information to the routing-doc about leading and trailing slashes in dynamic and rest-parameters. (as mentioned in #9103 (comment))