reactive-table icon indicating copy to clipboard operation
reactive-table copied to clipboard

Make i18n a weak dependency/optional?

Open SachaG opened this issue 9 years ago • 10 comments

Great work on Reactive Table! We're considering using it inside Telescope. But one issue is that Telescope already includes a different i18n package, tap:i18n.

So have you considered making i18n optional? Maybe anti:i18n could be a weak dependency, and only used if it's already in the project? Or maybe you could somehow let the end user provide their own i18n helper?

The anti:i18n package is not a ton of code so it's not a huge deal, but I just thought I'd raise the issue :)

SachaG avatar Mar 10 '15 00:03 SachaG

Thanks! I have thought about this but I'm not sure about the best way to do it.

One of the reasons for using an i18n library in the package is that the package can include its own translations. A weak dependency on anti:i18n wouldn't help that much unless I could also conditionally include the translation file based on whether anti:i18n was present. As far as I know that's not possible with package.js - do you know of a way? It would be easier with tap:i18n since it only loads the necessary translations, but it's a lot to set up in your app if you're not already using it. anti:i18n is just one line after you add the package.

An option for users to provide helpers would work, but they'd also have to write their own translations. I think I'd prefer an option to provide a language, and continue to use anti:i18n internally. Another possibility would be a weak dependency on tap:i18n, and have the package reactively update the internal anti:i18n language to match tap:i18n if it's there. It would be easier for tap:i18n users, but less flexible.

aslagle avatar Mar 10 '15 14:03 aslagle

My ideal solution would be to just replace anti:i18n by a weak dependency on tap:i18n. It'd take a bit of work but I feel like it'd probably be best to settle on a standard i18n package for every package to use, in order to avoid these situations.

I could contribute a PR if you want. Of course it's probably not super high priority, since there's only a couple strings translated right now.

SachaG avatar Mar 10 '15 21:03 SachaG

I'd rather not use tap:i18n within the package at this point, because setting up tap:i18n in an app would be the only way to translate the table controls. I could be wrong, but I think there are people using reactive-table in apps with a single language, who don't need a comprehensive i18n solution, just a way to use a single non-English language in the table. tap:i18n seems large and a lot of work to set up if you don't need it for other translations.

Using anti:i18n in the package, it's still possible to implement a way to use a language set from tap:i18n or other i18n libraries, but I don't think tap:i18n in a package is compatible with anything else. I'd be ok with switching if you can figure out how to do it in a way that also supports using the language set from anti:i18n, without adding tap:i18n to the app.

aslagle avatar Mar 11 '15 03:03 aslagle

Maybe @theosp can chime in?

SachaG avatar Mar 11 '15 03:03 SachaG

Hi,

A package can use tap:i18n without requiring apps that use it to have it installed. tap:i18n designed with a mindset of "package developers first" in contrast to most packages that are developed for apps hence you can look on it more as a library.

If tap:i18n is not installed in the app, it is considered "disabled" and only the fallback language - which is default to English will be in use, that is true that it's quite hard to change the fallback language in tap:i18n at the moment.

What do you think about the following approach:

Make anti:i18n a dep of tap:i18n so tap:i18n will take control over anti:i18n .

A new option will be added to project-tap.i18n "control-anti-i18n": true (default to true).

With this approach you don't need to install anti:i18n in your project if you use a package that uses it. i.e. tap:i18n users won't need to do the following: https://github.com/aslagle/reactive-table#internationalization .

I prefer the project-tap.i18n option approach over a weak dep approach, because it allow tap:i18n users to avoid installing anti:i18n at all on their apps.

Thanks, -Daniel MeteorSpark

theosp avatar Mar 11 '15 04:03 theosp

@hitchcott I know that you are using tap:i18n and tap:i18n-db together with reactive-table, what is your take on this?

theosp avatar Mar 11 '15 04:03 theosp

My (non-)solution right now is to hide all reactive-table UI that depends on anti:i18n :laughing:

@theosp I think the solution you've presented would work for some cases and solves the problem for other anti:i18n depending projects without them having to do anything. It's also worth checking if the locale keys are all the same or if you need to do any mapping (similar to my post on https://github.com/TAPevents/tap-i18n/issues/31).

In the long run, this does mean managing two different i18n packages, which isn't ideal. For me the ideal long term solution would be for reactive-table to use tap:i18n instead of anti:1i8n, although I am biased. Still, If I'm not mistaken, proper fallback support for non-english single-language apps would be needed to be supported in tap:i18n in order to make this totally transparent for non-english apps.

IstoraMandiri avatar Mar 11 '15 05:03 IstoraMandiri

I think tap:i18n controlling anti:i18n would work well. I was considering doing that in reactive-table, but it makes more sense to have it in tap:i18n. It just won't be able to use any of tap:i18n's lazy loading or translation override options. It might be better if tap:i18n could somehow be controlled by anti:i18n when it's in "disabled" mode, but that would probably be a lot more work.

aslagle avatar Mar 11 '15 13:03 aslagle

Anything on this ? I managed to translate the labels with

label: "#{TAPi18n.__('Some String')}"

tagrudev avatar May 21 '15 11:05 tagrudev

+1! It would be nice to settle on one i18n package. However, in this case here, with only 6 translations to be made, I don't think there's need for an i18n package at all...

I think the problem could be solved by just allowing these 6 labels to be configurable. Example:

   settings: function () {
       return {
           showFilter: true,
           fields: ['name', 'location', 'year']
           ...
           ...
          reactiveTableLabels: {
             filter: 'My app is english only. I will use default values',
             show: Tapi18n___("Yeah, I have this package installed"),
             rowsPerPage: i18n.map("I have this package installed too"),
             page: t9n("and also this one"),
             ...
          }

  };
}

juliomac avatar Aug 02 '16 04:08 juliomac