Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to type translation keys #1523

Closed
wants to merge 1 commit into from
Closed

Conversation

pierpo
Copy link

@pierpo pierpo commented Dec 8, 2020

This is a first step towards solving #1504
It allows you to type your translation keys freely. Which allows you to type them using the template literals of Typescript 4.1 for nested keys 馃槈 (see #1504).

Usage

In my project, I augment the module definition in a type definition filetypings/i18next.d.ts.

import 'i18next';

declare module 'i18next' {
  export interface TranslationKeys {
    keys: 'a' | 'b' | 'c';
  }
}

Then in the whole project, I may use the t function like this:

t('a'); // OK
t('wrong'); // Fails!

Same if I use react-i18next.

const Component = () => {
  const { t } = useTranslation();
  t('a'); // OK
  t('wrong'); // Fails!
  // ...
}

Advanced usage

Deeply typed keys using Typescript 4.1
import 'i18next';

import fr from 'src/i18n/translations/fr.json'

type PathOf<T extends object> = {
  [K in keyof T]: T[K] extends object ? [K, ...PathOf<T[K]>] : [K]
}[keyof T];

type SerializedPathOf<T extends object> = Join<Extract<PathOf<T>, string[]>, '.'>;

type Join<T extends string[], D extends string> =
    T extends [] ? '' :
    T extends [unknown] ? `${T[0]}` :
    T extends [unknown, ...infer U] ? `${T[0]}${D}${Join<U, D>}` :
    string;


declare module 'i18next' {
  export interface I18nTranslationKeys {
    keys: SerializedPathOf<typeof fr>;
  }
}

What's left to do

Well, it's just a first step. I'm open to suggestions: overriding the interface this way feels hacky, but I found no other way.

It also needs documentation somewhere.

  • only relevant code is changed (make a diff before you submit the PR)

  • run tests npm run test

  • tests are included

    • I would love to write a test, but adding an augmentation breaks the other tests since it's only done globally 馃槩
  • documentation is changed or added

@pierpo pierpo changed the title feature(types): add ability to type translation keys Add ability to type translation keys Dec 8, 2020
@pierpo pierpo force-pushed the patch-1 branch 3 times, most recently from 4a4bf70 to c010df2 Compare December 8, 2020 23:50
@coveralls
Copy link

coveralls commented Dec 8, 2020

Coverage Status

Coverage remained the same at 92.982% when pulling 6086577 on pierpo:patch-1 into ba564b3 on i18next:master.

index.d.ts Outdated
/**
* Gets the defined translation keys if they have been overridden, 'string' type otherwise
*/
type TranslationKeys = I18nTranslationKeys['keys'] extends string ? I18nTranslationKeys['keys'] : string;
Copy link
Author

@pierpo pierpo Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This raises a typescript error, so it should be done another way, but I have no idea how... I thought of adding { keys: string } in I18nTranslationKeys, but in that case we only will benefit the typescript autocompletion and no errors in case of incorrect keys. The most important is really the checks, not the completion.

It's best to solve the problem of this PR with a type augmentation so that the keys may be added 100% using typescript definitions.

Copy link
Author

@pierpo pierpo Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(btw, npm run tsc should be run in the CI as well, since current CI checks are green but they shouldn't)

Copy link
Author

@pierpo pierpo Dec 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a long thought about this, and it might have no solution as it is. I can't think of any way to augment a type strictly, and make it prevail over the default type (here, augment the keys with 'a' | 'b' and cancel out the default string type).

It works here as it is, but it needs a @ts-ignore, and I doubt that it's acceptable.

Maybe augmenting the type is a bad approach? In my opinion that's the most convenient solution as a consumer of the package, but I'm not sure we can implement it.

This is a first step towards solving i18next#1504
It allows you to type your translation keys freely. Which allows you to type them using the template literals of Typescript 4.1 for nested keys.
@jamuhl
Copy link
Member

jamuhl commented Dec 9, 2020

I'm not 100% sure why users insist on having the keys typed -> personally I add keys on the fly and let them added to the JSON by the saveMissing feature of i18next -> typed you have to manually add keys upfront to not break (or do I misunderstand how this works?)

@adrai
Copy link
Member

adrai commented Dec 9, 2020

//cc: @pedrodurek

@pierpo
Copy link
Author

pierpo commented Dec 9, 2020

I'm not 100% sure why users insist on having the keys typed -> personally I add keys on the fly and let them added to the JSON by the saveMissing feature of i18next -> typed you have to manually add keys upfront to not break (or do I misunderstand how this works?)

Indeed, such a feature makes it fine to work with the keys (I didn't know it to be honest, thanks!). But typing provides the following advantages:

  • I can't make a mistake. The second I make a mistake, my editor tells me, and the CI checks will be red.
  • I can safely remove outdated keys. If they're still being used, I'll get an error.
  • The auto-completion is great for devX. I can use existing keys very easily.

Also, it feels more natural to add my keys in the JSON then use them with the autocompletion 馃槉

@pierpo
Copy link
Author

pierpo commented Dec 9, 2020

Oh... I just noticed that this work has already been done on react-i18next directly!

i18next/react-i18next#1193

But I think it should be in here as well. It should be available to all frameworks!

@pedrodurek
Copy link
Member

Hey @pierpo thanks flagging that, as soon as we fixed all issues pointed out for the new types (almost there), we can certainly bring the types to here, if it's interesting for everyone.

@jamuhl
Copy link
Member

jamuhl commented Dec 10, 2020

I like the comment here: #1504 (comment) would be great to keep things opt-in and not again break existing builds (the start on react-i18next with the new types was rather though).

@stale
Copy link

stale bot commented Dec 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Dec 17, 2020
@stale stale bot closed this Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants