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: switch locale path loses query parameters #46

Merged

Conversation

BobbieGoede
Copy link
Member

Description

For some reason switchLocalePath is not working as expected in @nuxtjs/i18n, after debugging it seems like spreading the route here:
https://github.com/intlify/routing/blob/main/packages/vue-i18n-routing/src/compatibles/routing.ts#L301-L305

Does not work for @nuxtjs/i18n or maybe it is a Nuxt related issue, I noticed that the route object was a proxy and accessing each property did work. So I added a function to destructure each property by name instead of using a spread operator.

Unfortunately I don't know why the behaviour is like this, and I don't think I can add tests for this case as it may be Nuxt related.

This should resolve nuxt-modules/i18n#2354

Linked Issues

nuxt-modules/i18n#2354

Additional context

@BobbieGoede
Copy link
Member Author

This PR is what broke the copying of the route: nuxt/nuxt#21957

@kazupon
Copy link
Member

kazupon commented Sep 11, 2023

Thanks!

Oh, I forgot to update intlify/bridging deps... 😅
https://github.com/intlify/routing/actions/runs/6121817852/job/16654561435?pr=46#step:7:64

@@ -58,6 +58,21 @@ function split(str: string, index: number) {
return result
}

export function routeToObject(route: Route | RouteLocationNormalizedLoaded) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below is your Description on this PR:

Does not work for @nuxtjs/i18n or maybe it is a Nuxt related issue, I noticed that the route object was a proxy and accessing each property did work. So I added a function to destructure each property by name instead of using a spread operator.
Unfortunately I don't know why the behaviour is like this, and I don't think I can add tests for this case as it may be Nuxt related.

Let’s write the comment !, so maintainer can understand the background when we read this code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a short description now, hopefully it is informative enough.

Copy link
Member

@kazupon kazupon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just reviewed your PR.
Please check out!

@kazupon
Copy link
Member

kazupon commented Sep 11, 2023

One more thing, I've fixed e2e errors at #47
We need to update from upstream brach.

@kazupon kazupon added the bug Includes new features label Sep 11, 2023
@BobbieGoede BobbieGoede force-pushed the fix/switch-locale-path-query-parameters branch from 4b40f44 to 015a6d1 Compare September 11, 2023 09:24
@kazupon kazupon merged commit f6920dd into intlify:main Sep 11, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Includes new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

switchLocalePath does not keep query params in URL
2 participants