Add correct value for html lang attribute

[cfecherolle]

I made an implementation but I'm not sure if this is the right way to do things, feel free to comment :)

[daaru00]

I apologize.. I made a major change on PR #2 related to this switch and I think that can create a conflict...

I find a better method to change language using a router's beforeEach hook. This because global Vue mixins are applied to any component in the page and so created method hook is called at the end on every component creation. In my case I tried to found the first "entrypoint" of the application to immediately set the correct locale, but the check i18n.locale !== currentLocaleCode saved the situation.

I just tried to put a console log:

      if (options.pathAliases) {
        const pathAlias = options.pathAliases[i18n.locale];
        const lang = pathAlias || options.defaultLocale;
        console.log('head set'); // <------- here
        head.htmlAttrs = { lang: lang };
      }

then, when I load the homepage:

html htmlAttrs is called around 40 times. I don't think it inflicts significantly to performance, but can be optimized.

I think the solution of beforeEach hook can solve the problem and let you to inject this logic in the first page load and never again.

Is it ok for you if I merge my PR #2 before yours?

[cfecherolle]

Yes! Of course, you can merge PR #2 before, I'll get back to this one using beforeEach when #2 is merged :)

[daaru00]

Ok @cfecherolle, PR #2 merged!

You now should be able to use routers hook rebasing your branch from master (or merging, up to you). Sorry again to throw a spanner in your works :sweat_smile:

[cfecherolle]

When using the beforeEach hook, I'm having trouble updating the lang value when changing language (without refreshing the page, I mean):

  // Set the correct lang attribute for html tag using the current locale
  router.beforeEach((to, from, next) => {
    if (options.pathAliases) {
      const pathAlias = options.pathAliases[i18n.locale]
      const lang = pathAlias || options.defaultLocale

      head.htmlAttrs = {lang: lang}
    }
    next()
  })

Works fine on first load, then gets triggered again when I click a flag in my language selector but the lang attr stays the same in the DOM and is not updated. Do you have an idea maybe about how/why?

[daaru00]

yeah, same here.

I've just putting a dummy console.log after head property set seems the code is correctly execute even after a locale switch. Something cross my mind.. maybe, switching to a different page, trigger Vue Router to replace the entire view object, instead, changing locale only pass a different option (the locale code into context object) to the current page. The head object seems correctly set but maybe nothing is watching the change and didn't update the markup.

Also tried the old method:

  Vue.mixin({
    created: function () {
      // Set the correct lang attribute for html tag using the current locale
      if (options.pathAliases) {
        const pathAlias = options.pathAliases[i18n.locale];
        const lang = pathAlias || options.defaultLocale;

        head.htmlAttrs = { lang: lang };
      }
    }
  })

and I experiencing the same issue.

[cfecherolle]

I found a workaround by:

• using head.htmlAttrs in beforeResolve hook (this is for setting the attr on page initial load)
• using vanilla JS to update the attr on route change in an afterEach hook

It's not very elegant, but at least it's functional :sweat: I've pushed the new commit, tell me what you think (I haven't found any other solution yet)

[daaru00]

Awesome, it works great!!

It's not very elegant, but at least it's functional sweat

it's not so bad, it's robust and fully compatible cross-browser. You also add a comment do not forget, that's fine for me :+1:

One question, why did you check options.pathAliases settings? I think is related to ISO 639-1 format (ref) required for that tag. Just thinking about someone using a configuration like this:

module.exports = {
  plugins: [
    {
      use: "gridsome-plugin-i18n",
      options: {
        locales: [
          'it',
          'fr'
        ],
        messages: {
          'it': require('./src/locales/it.json'),
          'fr': require('./src/locales/fr.json')
        }
      }
    }
  ]
};

could respect the format without the need of pathAliases settings. Vue i18n seems compatible, (even they use "en" and "ja" in examples snipped).

Just asking, about the rest, looks great for me!

[cfecherolle]

Bad news: my workaround works great using gridsome develop but when I try gridsome build, it fails miserably:

ReferenceError: document is not defined
    at assets/js/app.6f49e0a9.js:8624:7
    at conditionalLangAttrUpdate (gridsome-plugin-i18n/gridsome.client.js:72:6)

I will try to fix this or find a better way...

And sure, I went a little bit overboard with the pathAliases, we should just use the locale directly :) I'll update this in a new commit.

[cfecherolle]

Nevermind, I found a way: to avoid SSR for this part of the code (thus, not finding document ) I added a check with if (process.isClient) and now the build also works!

I'll let you do the review, now that it seems to work on my side :)

[cfecherolle]

@hacknug I applied your feedback, could you check? :) Thanks!

[cfecherolle]

@daaru00 Shall we merge this? Or maybe someone should do a final review?

[cfecherolle]

Thanks! I bumped the version in package.json and package-lock.json and merged.