aurelia / i18n

A plugin that provides i18n support.
MIT License
93 stars 70 forks source link

options.attributes property is applied twice, is incorrectly documented, and plays poorly with typescript #222

Closed dkent600 closed 7 years ago

dkent600 commented 7 years ago

In this article you call for adding ['t', 'i18n'] in two different places (which has strong code smell).

One of the two places is where setting an attributes property on the i18n options object that is passed to setup and then directly on to i18n.init(). Your docs state: "adapt options to your needs (see http://i18next.com/docs/options/)".

The code smell was bothering me -- I want to write cleaner code to use the plugin, so I went to the above-referenced i18next documentation page, I looked in the i18next d.ts, I looked in the i18next and aurelia-i18n source code, and the only place attributes appears to be used is in the aurelia-i18n plugin. It is not part of i18next, despite what the aurelia-i18n docs imply.

I would like to request that:

1) the docs be clear that this property is not part of i18next (so people like me don't waste so much time trying to figure out what it is used for -- why is it needed twice?), be clear about what it is used for, and indicate whether there is a default when the array is not provided. 2) best: that everything required of ['t', 'i18n'] be done via a single API mechanism, not two.

Finally, there is a potential issue here with typescript. (I say potential because I haven't figured out how to access the i18n Options type; it doesn't appear to be exported, or is done so in a way I don't yet understand (like a namespace)). But if I can access it, and I apply it, the compiler will complain that attributes property is not part of the Options interface.

zewa666 commented 7 years ago

Hi @dkent600. Yes the attributes option is an aurelia i18n custom one. Its similar to the selectorAttr used by the jquery implementation, but instead allows multiple aliases, and no official part of i18next itself. The hint about adapt options to your needs was refering to other settings like namespaces, interpolations etc.

Yes you're also right that it currently feels like a code-smell having to register your attributes twice. The reason is that the attributes option will be used to do the default wire up, also as you've mentioned with the defaults ["t", "i18n"]. The section TCustomAttribute.configureAliases(aliases); on the other hand needs to be done, before the plugin intialization begins, since this is right after view templates get processed and thus the new aliases aren't recognized soon enough.

Regarding TypeScript, as you mentioned, you won't find those in the official typedefs since its a custom option by aurelia i18n.

I've created a PR to address your points in the documentation, so if you could take a look at them and give me some feedback, whether you think this is enough we can get that merged in.

Thanks for taking the time to report your issue and sorry for the inconvenience

zewa666 commented 7 years ago

Closed due to referenced PR being merged