skolmer / es2015-i18n-tag

ES2015 template literal tag for i18n and l10n (translation and internationalization)
http://i18n-tag.kolmer.net
MIT License
191 stars 13 forks source link

Typescript support #11

Closed MastroLindus closed 8 years ago

MastroLindus commented 8 years ago

Hi, Awesome project!

Would you consider supporting typescript? Since it provides template strings by default in the language it shouldn't be much work.

I noticed that you have a declaration file index.d.ts in your lib folder, but for typescript to find it you need to either have it in the root of the project, or have a "typings":"./dist/lib/index.d.ts" entry in your package.json. If you add that, typescript will pick up the types.

However even with that fixed, the typings for typescript are slightly wrong since in typescript a template string is of type "TemplateStringsArray" and the current declaration file expects strings.

skolmer commented 8 years ago

Thank you! I don't have much experience with typescript. I'm trying to solve this but can't find any good examples. The problem is that i18n can be used in different ways e.g. i18nmystring, `i18n('group')`mystring or i18n('group', 'config')mystring``. I don't get any intellisense for the first example in vscode if I try the following .d.ts. I also don't get any intellisense for the literals param:

export default function (group?: string, config?: string) : (literals: TemplateStringsArray) => string

maybe you can help with more details or provide a pull request?

MastroLindus commented 8 years ago

Thank you for such a quick response :)

For solving the issue of the function being used in different ways, I think typescript allows to provide different overloads of the same function for that. Look at https://www.typescriptlang.org/docs/handbook/functions.html in the overloads section.

I am not very familiar on how typescript (or javascript) handles the tagged template strings the way you use it in the library (i18ntest) so I am not sure what the proper signatures should be.

Having a quick look, it looks that something like export default function i18n(literals: TemplateStringsArray, ...values: string[]) : string; export default function i18n(group: string, config?: string) : (literals : TemplateStringsArray, ...values : Array) => string;

will actually work (it does for me)

An additional suggestion would be to make all the config-related fields optional (all the stuff in DateConfig, NumberConfig, Config that is allowed to not be specified) and add semicolons at the end of the declarations

Finally, even if this is somehow personal, I prefer to not have default exports if I am already exporting other stuff, so I would make i18n a normal export (but again, this is completely subjective)

skolmer commented 8 years ago

Thanks a lot! This was already very helpful. I updated the definition file and also added the typings setting to package.json. Please check out v1.1.2 and let me know if this works for you.

https://github.com/skolmer/es2015-i18n-tag/releases/tag/v1.1.2

MastroLindus commented 8 years ago

You are welcome :)

Now it works already much better out of the box. Some minor feedback: -the function docs should be updated as well, otherwise the IDE will show you the doc for the wrong overload (each overload should have its own doc comment on top with the proper variables listed) -config.standardFormatters should probably be optional as well, not only its types (otherwise you still need to specify it, even if it is just empty) -minor things already mentioned above: semicolon at the end of each declaration, and (if you are not against it) removing the export default for just an export (I find import i18n, { i18nConfig } from "es2015-i18n-tag"; much uglier than import {i18n, i18nConfig } from "es2015-i18n-tag"; but again, this is subjective)

skolmer commented 8 years ago

I agree, standardFormatters prop should definitely be optional. In fact it is in JS so it should be the same for typescript users. I will change that.
I will also fix the documentation, sorry for that and thanks for testing and your valuable feedback.

Regarding the export. I decided to export i18n as a default because this is the tag function itself and the most common import will be import i18n from 'es2015-i18n-tag'. There are even usecases (like build time translation) where you won't need i18nConfig or i18nGroup so these are optional imports. Anyway, this would be a breaking change and I don't want want to break things for those that are already using this library in their JavaScript projects.

MastroLindus commented 8 years ago

It makes perfectly sense :) Thanks!

skolmer commented 8 years ago

Do you see any autocompletion if you type i18ntest``? I use vscode and I'm not sure if this is an issue in vscode intellisense or related to the typescript definition. It looks like this is a vscode issue because it is not marked as an error. What IDE are you using?

skolmer commented 8 years ago

standardFormatters are now optional. released in v1.1.3

MastroLindus commented 8 years ago

I use vscode as well, and I get autocompletion for the function doc between the two overloads