core: improve Translator type for strict TypeScript compatibility

[lucas-koehler]

Fix #2528

• Replace overloaded Translator type with a generic conditional type
• The generic type allows inferring the return type from the handed over default message. I.e. a undefined defaultMessage results in string | undefined while a string default message results in string.
• Add createTranslator helper to allow implementation without type assertions under strictFunctionTypes and strictNullChecks.

[netlify]

✅ Deploy Preview for jsonforms-examples ready!

Name	Link
🔨 Latest commit	6d9deee
🔍 Latest deploy log	https://app.netlify.com/projects/jsonforms-examples/deploys/69bd10638b1a61000868485a
😎 Deploy Preview	https://deploy-preview-2562--jsonforms-examples.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[coveralls]

coverage: 82.869% (+0.02%) from 82.852%
when pulling 6d9deee on lk/issues/2528-translator-typing-issues
into effb45c on master.

[sdirix]

I understand the intention of the change but personally I would like to do a different approach.

The PR solves the typing issue by force-casting a simplified translator method to our own. However this means that a "wrongly" implemented translator which takes in a "string"-default parameter but still returns "undefined", is accepted by our official typing and used, without further checking, as if the requirement holds true.

I see two different approaches:

• Either extend "createTranslator" to not be a no-op but actually enforce the invariant given above, or
• Get rid of the fancy typing, get rid of createTranslator and just use the simplified type in all call sites. In some places we then slightly need to do a bit more checking but should be manageable.

What do you think?

[lucas-koehler]

@sdirix Thanks for the feedback!

While I generally like the narrowing effect of the generic type, I see your concern. I honestly did not think of the problem of implementation faults that "illegally" return undefined even if a default value is given.

• Either extend "createTranslator" to not be a no-op but actually enforce the invariant given above

That might be a nice idea. We can just return the default value in any case if the called translator returns undefined. We could even extend this to also apply to null for non-strict or JS environments. Is this what you meant?

• Get rid of the fancy typing, get rid of createTranslator and just use the simplified type in all call sites. In some places we then slightly need to do a bit more checking but should be manageable.

I'm not sure what exactly you mean with that. Do you mean simplifying the Translator type to the following?

export type Translator = {
  (id: string, defaultMessage?: string, values?: any): string | undefined;
};