Implement Types for native

[rgant]

Bad types like any are so much worse than no types at all. But at least this would be correct types (mostly). #64

[n1k0sv]

@rgant Awesome work! Can you please sign the Transifex CLA as instructed here, so we can accept your contribution?

[rgant]

I have emailed in the signed CLA.

Because of issues with the @transifex/angular implementation at scale I am having to rewrite the Angular Transifex implementation to improve performance. Fighting with the :any types in the native implementation has been a roadblock to my progress. I see you just released today, but it would be really (really really) awesome if you could either merge this or remove index.d.ts from the native package. (So I can implement my own types. TypeScript does not support overriding packaged types. So when you package :any types [telling TS to not be helpful] that results no one being able to write better code.)

I my intent here is not to be rude, and I apologize if I am coming off that way. This is a stressful time for my work where we are trying to launch translations for our language heavy Angular app and have run into race conditions and problems with translations in our app.

[rgant]

Noticed at least two bugs I should fix:

1. sanitize is not a ITranslateParams recognized by the native API. It is Angular only.
2. Since ITranslateParams can have any ICU expression variables added to it it needs an index signature.

[rgant]

Same issue for _inline as for sanitize

[pablotransifex]

@rgant Hello, can you please give us some details of what you are trying to achieve when declaring some Native Angular SDK interfaces and properties in the JS Native base package?
Some interfaces (e.g. ITranslationServiceConfig) have meaning only in the Angular SDK, and some properties as well (e.g. sanitize).
Can you maybe give as some examples of your problem to better understand what is needed? Thanks!

[rgant]

The real problem is that the dual subscriptions in TComponent, TranslatePipe and UTComponent in the @transifex/angular package produce race conditions that mean that my app ends up with some text in English and some text in Spanish when switching between languages.

I rewrote those Components (Pipe, and Service) to use Angular best practices and that resolved the issues. However that results in numerous complaints from my code linting and build system because of the index.d.ts declaration file you have in @transifex/native package. (:any is a terrible type to declare, and by doing so inside your package only @transifex/ can fix the issues with the types.) I cannot submit those improvements to @transifex/ without first fixing the types.

I did make notes regarding the problems with the interfaces in this ticket last night. If you are willing to merge this pull request then I will update it to resolve those problems. If you are unwilling to merge this pull request and unwilling to remove the index.d.ts from the @transifex/native package then I will be forced to re-implement the native package. I believe I can make significant improvements to that package's performance, but having appropriate types is the first step.

If you delete the index.d.ts file then I will be able to submit these types to DefinitelyTyped and the parts of your community that care about type safety and TypeScript will be able to benefit. But ideally your package would have appropriate types.

[rgant]

ITranslationServiceConfig actually has meaning in these types now. That type is implemented by TxNative to ensure that the config and the class match. Also used by the init and createNativeInstance methods to properly declare what values are acceptable for initializing and creating new TxNative instances.

[pablotransifex]

@rgant Thank you for the feedback, I would like to see the changes you did in the Angular SDK package, for sure any improvement is more than welcome!

As for the ITranslationServiceConfig, I was referring to the fact that there is no TranslationService in the JS Native base package, so probably we can keep it if gives a meaning as you said.

[n1k0sv]

@rgant The plan is to move forward with this PR. Can you please rebase the branch and we will take a final review tomorrow, and we are good to go.

[rgant]

Is the new name better?

[rgant]

The failures in the Angular tests are something I was half expecting. I can clean those up, but I will need time as I am currently working on different projects.

[n1k0sv]

Thanks @rgant, we need to fix these before we can merge. We will take a look to see if we can help as well.

This is an important update and we really appreciate the effort you are putting into this 🙏

[pablotransifex]

@rgant Hi, I made some fixes for the Angular SDK tests, now are passing :)

cc @nbasili

[n1k0sv]

@rgant Released as 5.1.0 🎉

[rgant]

Awesome thanks so much!