react-native-globalize icon indicating copy to clipboard operation
react-native-globalize copied to clipboard

[Proposal] useGlobalize generic type

Open fend25 opened this issue 4 years ago • 7 comments

Hello!

Didn't you think about some way to specify possible list of IDs? For example:

// it is possible to build programmatically from your translations object, even nested
type AllowedIds = 'guests' | 'yes' | 'no' 

const formatMessage = useGlobalize<AllowedIds>()

formatMessage('yes') // ok
formatMessage('maybe') // compile time error, not runtime

possible generic could be like that (orig code):

export interface Globalize<IDType extends string =string> extends GlobalizeConfig, GlobalizeHelpers, Formatters {
  // ...
  formatMessage(
    id: IDType | IDType[],
    values?: string[] | Record<string, string>,
    options?: MessageFormatterOptions,
  ): string;
  // second overload same way
  // ...

It won't break code for users who don't use this generic (because the fallback is the old type: string).

It won't be useful for users who slash-combined ids: 'party/guests'. But I think than benefit gain worth it, because it makes the lib really type strong (strings are the weak place almost always). So, for users who prefer to use complex paths (via slash), nothing is changed, meanwhile for users who don't want to rely on strings it would be a huge benefit.

fend25 avatar Mar 18 '20 07:03 fend25

Definitely open to improving the experience around message identifiers. I'd prefer something a little more "automatic" though - perhaps something that works with loadMessages.

joshswan avatar Mar 18 '20 19:03 joshswan

Yeah, I was thinking about some way to make automatic check, but now there is one circumstanse: nested path (i.e. 'foo/bar'). Unfortunately, in the current Typescript version can not infer the type of string concatenation. Example:

const a: 'a' = 'a'
const b: 'b' = 'b'
let c = a + b // type is string, not 'ab'
c = '123' // ok, no any errors

I've found an article about it https://medium.com/@flut1/deep-flatten-typescript-types-with-finite-recursion-cb79233d93ca , but there is no way to get string type.

I do automatic id type inferring via such trick:

const one = {/* translations */}
const two = {/* translations */}

const dictionary = {
  ...one,
  ...two,
}

const IDictionaryId = keyof typeof dictionary 

So, I can check whether the string is valid id at compile time. And for the complex IDs there should be some string concatenation, but I am really not sure whether there is a TS magic way to do it. I think now TS can not infer this, meanwhile building an object of all keys in some recursive function will cause string concatenation, which brake TS' inferring of object to it's interface.

Also there is a one more problem: messages object now is object of dictionaries per locale. It's assumed that all per-locale dictionaries should have the same shape, but in TS we can not rely on assuming, we have to definetely prove that they are the same. So, we have to do some checking of dictionariy sub-objects relying on one of the sub-objects. I am not sure that TS allows such trick and that the TS should do it, but it's just my thoughts.

I am not against of automatic ID validation, I am rooting for it at all. I am just trying to discuss some problems that we have and hear your thoughts about it.

Thank you anyway for your great work and your attention.

fend25 avatar Mar 19 '20 18:03 fend25

Yeah I think we'll just have to ignore typing the nested/string syntax as TS doesn't support it. However, the array syntax for nested keys (e.g. ['foo', 'bar']) could be strongly typed:

export type MessageData = Record<string, Record<string, string | Messages>>;

export interface Globalize<
  M extends MessageData = { [id: string]: Record<string, string | Messages> }
> extends GlobalizeConfig, GlobalizeHelpers, Formatters {
  formatMessage<
    Locale extends keyof M,
    ID1 extends keyof M[Locale]
  >(
    id: [ID1],
    values?: string[] | Record<string, string | number>,
    options?: MessageFormatterOptions,
  ): string;
  formatMessage<
    Locale extends keyof M,
    ID1 extends keyof M[Locale],
    ID2 extends keyof M[Locale][ID1]
  >(
    id: [ID1, ID2],
    values?: string[] | Record<string, string | number>,
    options?: MessageFormatterOptions,
  ): string;
  // support for maybe 5 levels?
}

It's definitely not pretty, but using Globalize<typeof myMessagesObj> and the array syntax seems to work quite well this way. However, the following would still have to be considered/improved:

  • How should it behave with differently-shaped locales? The solution above only permits keys that are common to all.
  • How many levels of nesting to actually support?
  • How should single first-level keys be handled (i.e. should writing ['key'] be necessary)?
  • Assuming we go down the path of strongly typing the array syntax, should that be the preferred syntax in the docs?

Anyway, definitely room for improvement, but neither the article you linked nor various other libraries gave me any inspiration for something more "magical". What are your thoughts? Should one of us open a PR so we start playing around with solutions?

joshswan avatar Mar 19 '20 20:03 joshswan

Oh, you did a deep investigation!

So, you pose an eternal question - simplicity of API vs it's accuracy and complexity.

Basing on my experience, almost all TS typings that was trying to cover universal cases became too complex and not neat.

I don't believe there is a chance to resolve all these issues (at least the 4 you have posed) without overcomplication the API.

My opinion: for only this case, for such deep and hacky TS magic - don't chase the universal solution. Just provide some robust and clear-how-to-use solution. There is too much options and cases and users can pick their way themselves. Still I think we shouldn't complicate API for the less percent of users at the cost of API complication for the most part. I prefer to follow the unix way - your lib does the translations, not the TS inferring serivce. And let it do the translations, the only one thing clear and robust.

You make this lib for fun. And you do this really well. I am rooting for the simplest and the cheapest way to reach the enough quality.

I am just adviser, the decision is yours :)

PS. sorry for language mistakes and for verbosity. Just don't know how to express simpler.

fend25 avatar Mar 19 '20 22:03 fend25

Just released your original proposal in v4.2.0. It's definitely the simplest and most flexible solution on the table.

We use this library internally at my company now (which was the main motivation behind the TypeScript rewrite), so I'll let people play around with this solution and see if more improvements (or complications) are warranted. Leaving this issue open in the meantime.

Thanks for your input on this one! And definitely let me know if you see room for improvement :)

joshswan avatar Mar 20 '20 01:03 joshswan

Thank you!

Definetely I will let you know if I'll have any ideas.

Now I am afraid that my idea cause API break: it's not possible to set type of useGlobalize globally, so the way is to provide some factory:

// globalizeFactory.ts
const {useGlobalize} = loadMessages(SomeMessageObject)
export useGlobalize

// someComponent.tsx
import {useGlobalize} from 'src/globalizeFactory.ts' //not from the lib

fend25 avatar Mar 20 '20 14:03 fend25

I wouldn't call it a "break" since the original string typing still applied. However, to avoid people getting their types messed up, I just released v4.2.1 which reverted the changes. And I re-released the current solution as v4.3.0-alpha.0.

The feedback I got so far was:

  • this solution works okay in specific areas of the application (e.g. a feature folder with related components and messages so importing the messages type and using it whenever using useGlobalize isn't too painful - though some question whether the benefit is worth it)
  • as you mentioned, it doesn't work globally, and continuously importing a global messages object type is annoying

The global issue could be resolved by improving the context provider, but ideally I really want something that doesn't get in the way.

joshswan avatar Mar 20 '20 17:03 joshswan