-
-
Notifications
You must be signed in to change notification settings - Fork 489
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 adding duplicated routes when users were using router.extendRoutes
#1281
base: v7
Are you sure you want to change the base?
Conversation
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
not stale |
This change is preventing prefixes from being added in some (all?) cases. See the unittesting results. Also I'd like to see at least one new test added to test the case that you are fixing. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Not stale, we have the same issue. Any news on this PR or something others could help with? |
@Nakroma I'd say adding a failing test to demonstrate how this is broken in the current version of the module would be extremely helpful and get us one step closer to implementing a proper solution for it. Once we have that in place we can re-implement some of the changes in this PR in a way it doesn't affect any of the existing features of the module (this PR currently is making some of the tests fail). An explanation about your use case and how the current implementation is not working for you would also be helpful in case we need to consider a different scenario from the one I tried to explain in #1280. I've had this in production since I opened the PR and it works fine for my use case. There's some issues with some routes on the sitemap but I haven't been able to put in the hours to research what's really going on and who's the culprit. |
When trying to replicate our issue for the failtest I took another look at our problem and it turns out it was an issue with nuxt itself that was causing the route duplication, i18n was just obfuscating it, so our problems seems to be a different one from the one mentioned in #1280 |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Not stale |
This shouldn't be merged without review for obvious reasons. An explanation for the solution provided in this PR can be found in issue #1280.
The idea here is to check if the route already contains
routesNameSeparator
. In that case it probably means the user already anticipated the lack of support for this and maybe other features and we can (hopefully and) safely skip patching the route object with redundant data.This was adding the same route a couple times so I added a conditional to check if a route with the same name already exists. If that's the case, we only push the new route when it has a
children
property because from what I saw, the module only localizes dynamic children routes in those cases.Works fine on my local enviroment but I'm pretty sure this isn't taking into account all of the cases this module supports. Let me know if you want me to make any changes or add any extra details.
Closes #1280.