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

[localize] Make all components forward compatible with dialects #100

Closed
bashmish opened this issue Jun 14, 2019 · 5 comments
Closed

[localize] Make all components forward compatible with dialects #100

bashmish opened this issue Jun 14, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@bashmish
Copy link
Contributor

bashmish commented Jun 14, 2019

I want all components to work with all locales for languages we have in ./translations/, while at the same time support polymer-cli for the ones we manually define in static get localizeNamespaces().

It is easier to illustrate my idea with the code.

Imagine we have ./translations/en.js and such code:

static get localizeNamespaces() {
  return [
    {
      'lion-component': locale => {
        switch (locale) {
          case 'en-GB':
          case 'en':
            return import('../translations/en.js');
          default:
            throw new Error(`Unknown locale: ${locale}`);
        }
      },
    },
    ...super.localizeNamespaces,
  ];
}

Then this code will work for en-AU and en-US thanks to just one change and this feature of localize:

static get localizeNamespaces() {
  return [
    {
      'lion-component': locale => {
        switch (locale) {
          case 'en-GB':
          case 'en':
            return import('../translations/en.js');
          default:
            return import('../translations/${locale}.js'); // changed
        }
      },
    },
    ...super.localizeNamespaces,
  ];
}

This implies that fact that 2 requests will be needed for locales which don't have a file in ./translations/, because this is how this feature works. But we knew that when we designed it and that was a trade-off, it is just weird we are not using it since then ourselves.

This forward compatibility is useful for people using native dynamic imports or/and webpack for bundling, because they can then use existing languages that we already support, e.g. English, for other countries. For example, most of changes like this for "en-PH" will not be needed for them, unless they also need to support polymer-cli.

I would say the fact we are not forward compatible with modern standards is a bug.

@bashmish bashmish added the bug Something isn't working label Jun 14, 2019
@daKmoR
Copy link
Collaborator

daKmoR commented Jun 14, 2019

for my understanding, you want to do this replacement everywhere?

-          default:
-            throw new Error(`Unknown locale: ${locale}`);
// -----------------------
+          default:
+            return import('../translations/${locale}.js');

@LarsDenBakker
Copy link
Contributor

LarsDenBakker commented Jun 14, 2019

This should work, I think polymer-cli just leaves dynamic imports with expressions untouched.

Also what's important here I think is that the dynamic imports and the translation files are statically analyzable. That way we can create build-time optimizations per application to smooth out performance penalties.

For example I've been experimenting with a webpack plugin which throws away language files for locales we don't use, and creates entrypoints with inlined translations for each locale for performance. We could make it so that it checks during the build whether there is a locale file available, and otherwise imports the language file. That way it will do 2 imports during development, but 1 import after building for production.

@bashmish bashmish changed the title [localize] Make all components forward compatible with new locales [localize] Make all components forward compatible with dialects Jun 14, 2019
ovidiu1 added a commit to ovidiu1/lion that referenced this issue Jun 15, 2019
@tlouisse
Copy link
Member

Good idea.

Questions:

  • Do we also need a fallback for languages not found?
  • Will we ever be able to resolve the FIXMEs? Would it maybe be an idea to autogenerate this switch statement as a prepublish script, so we keep the source code succinct and maintainable?

@bashmish
Copy link
Contributor Author

Fixed in #102

@bashmish
Copy link
Contributor Author

@tlouisse good questions

Do we also need a fallback for languages not found?

Like English for whatever was not found?

Will we ever be able to resolve the FIXMEs? Would it maybe be an idea to autogenerate this switch statement as a prepublish script, so we keep the source code succinct and maintainable?

Yes, we discussed this already a while ago when we were still on bower. Yeah, looks like a great idea to me to just use NPM prepublish for that, should be doable and simpler than solutions discussed previously. I will create an issue for that!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants