Skip to content
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: custom routes with optional params adjusted incorrectly #1243

Merged
merged 2 commits into from
Jul 22, 2021

Conversation

rchl
Copy link
Collaborator

@rchl rchl commented Jul 22, 2021

Don't use ufo on Vue Router route definitions as those are not
a normal paths, or not always at least.

Fixes #1242

rchl added 2 commits July 22, 2021 13:37
Don't use ufo on Vue Router route definitions as those are not
a normal paths, or not always at least.

Fixes #1242
@rchl
Copy link
Collaborator Author

rchl commented Jul 22, 2021

@divine I guess it wasn't correct or necessary to use ufo in makeRoutes. I don't think it was a fix for anything.

@divine
Copy link

divine commented Jul 22, 2021

@divine I guess it wasn't correct or necessary to use ufo in makeRoutes. I don't think it was a fix for anything.

Well, I just thought it would be better to handle via ufo instead of manually appending trailing slashes.

Sorry about that!

@rchl
Copy link
Collaborator Author

rchl commented Jul 22, 2021

@divine I guess it wasn't correct or necessary to use ufo in makeRoutes. I don't think it was a fix for anything.

Well, I just thought it would be better to handle via ufo instead of manually appending trailing slashes.

Sorry about that!

Not blaming, just discussing to ensure that I'm not reverting actual fix for something.

@divine
Copy link

divine commented Jul 22, 2021

Not blaming, just discussing to ensure that I'm not reverting actual fix for something.

Got it, the actual fix was in https://github.com/nuxt-community/i18n-module/blob/0527d63b99cf30fbe71bd62ded731de3a86798fc/src/templates/plugin.routing.js#L91

You were right that there are too many options and not enough tests for each option which created a regression without knowing it.

Thanks!

@rchl rchl changed the title fix: custom routes with optional params broken by the module fix: custom routes with optional params adjusted incorrectly Jul 22, 2021
@rchl rchl merged commit 203f3db into master Jul 22, 2021
@rchl rchl deleted the fix/route-with-opt-params branch July 22, 2021 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Routes ending with an optional slug are not working anymore
2 participants