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

localePath remove unused params that should be sent as props to router-view #728

Closed
2 tasks done
mrleblanc101 opened this issue May 22, 2020 · 11 comments · Fixed by #729
Closed
2 tasks done

localePath remove unused params that should be sent as props to router-view #728

mrleblanc101 opened this issue May 22, 2020 · 11 comments · Fixed by #729

Comments

@mrleblanc101
Copy link
Contributor

Version

nuxt-i18n: 6.3.0
nuxt: 2.10.1

Nuxt configuration

mode:

  • universal
  • spa

Nuxt-i18n configuration

            {
                // Options
                vueI18nLoader: false,
                detectBrowserLanguage: false,
                locales: [
                    {
                        code: 'fr',
                        name: 'Français',
                        iso: 'fr-ca',
                    },
                    {
                        code: 'en',
                        name: 'English',
                        iso: 'en-ca',
                    },
                ],
                defaultLocale: 'fr',
                seo: false,
                vueI18n: {
                    silentTranslationWarn: true,
                    messages: lang.translations,
                },
            },

Hi,
I'll start by mentioning I'm using router-module with Nuxt and that this issue is caused by the interaction of the 2 modules, but the problem is with nuxt-i18n and not router-module.

What is Expected?

app.localePath() should only append the language to the name key of the object, it should not transform the object as a URL string as you can pass params that won't be in the URL, but that gets passed to the router-view when using the props option. More info here.

What is actually happening?

app.localePath() transform the router-link object to a string which remove all params that are send but are not displayed in the URL.

For exemple:

app.localePath({
    name: 'parcours/stop',
    params: {
        stop: 'hollywood-blvd',
        stopId: '1234',
    }
})

turn into myapp.com/parcours/stop/hollywood-blvd which looks good, but now my router-view won't receive the stopId as a props.

If you have this problem, I found that I could access the locale directly and append the language to the name of my route like so instead of using the localePath fonction:

app.$router.replace({
    name: `parcours/stop___${app.$i18n.locale}`,
    params: {
        stop: 'hollywood-blvd',
        stopId: '1234',
    }
})

I'm not sure if the best way to solve this problem is to change the way the current localePath fonction works or add a new localeObject (example) fonction

@rchl
Copy link
Collaborator

rchl commented May 23, 2020

It would be helpful if you would set up a repro project on codesandbox for example. I guess router-module configuration is also relevant here.

@mrleblanc101
Copy link
Contributor Author

Yes, I did not have time to create one yesterday.
Here it is: https://codesandbox.io/s/github/mrleblanc101/nuxt-i18n-localePath-bug/tree/master/

@mrleblanc101
Copy link
Contributor Author

As you can see, when you click «params bug», the stopId param is missing (the object is printed bellow the buttons) because it uses localePath. If you click on «params ok», the stopId param is passed because we translate the name key of the param object.

@rchl
Copy link
Collaborator

rchl commented May 24, 2020

I don't see how I could make it work with localePath. As you've noticed yourself, it returns a string. This is so that the result works in various contexts like plain links, meta tags, and others.

So it's not really equivalent to using router.push with an object. It's more an equivalent to router.resolve and getting full path of resolved route.

Adding a new API to achieve what you want would probably be the best option. For example just add a way to resolve the name of the localized route so then you could construct a name property that you would use in the object passed to $router.push().

Maybe getLocalizedRouteName(name, locale)?

@mrleblanc101
Copy link
Contributor Author

I understand all that.
But:

  1. Why dumb down localePath to return a string instead of a translated object ?
    I don't think this is the job of i18n as you could already transform the route object to a route string using $router.resolve

  2. Should there really be 2 API to achieve the same result ? Maybe localePath should have a deprecated warning and be removed in the next major version to keep only one API ?

  3. The idea behind localePath is that it work automatically, you do not the to send it the locale every time.

  4. Also localePath vs getLocalizedRouteName seems like 2 very different API name for achieving almost the same thing. Why not something like localeObject

  5. It would be easy to create a fallback inside the new API if the object does not have a name key for some reason if localePath support translating route object without name (I'm not sure if this is currently possible)

@rchl
Copy link
Collaborator

rchl commented May 24, 2020

Why dumb down localePath to return a string instead of a translated object ?

If we would return object, people would need to do localePath('about').fullPath instead in some contexts (plain links or meta tags). It wouldn't be a very intuitive API.

So the localePath API won't change but I guess it makes sense to introduce something like localeRoute instead of adding getLocaleRouteName. It would basically be the same as localePath code-wise but return resolved route instead of route.fullPath.

@rchl
Copy link
Collaborator

rchl commented May 24, 2020

What do you think about updated #729 @mrleblanc101 ?

@mrleblanc101
Copy link
Contributor Author

Looks good to me. I would've simple changed getLocalizedRouteName to use the currentLocale by default too if it's still available globally

@rchl
Copy link
Collaborator

rchl commented May 25, 2020

I haven't added getLocalizedRouteName in the end, just localeRoute.

@mrleblanc101
Copy link
Contributor Author

Oh I must be confused, I thought I saw that localeRoute was calling getLocalizedRouteName when I looked at the commits yesterday.

@mrleblanc101
Copy link
Contributor Author

Also, thank you for the very fast follow up. I see this is already merged and published on NPM ! 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants