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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(types): allow readonly namespaces in useTranslation #1339

Merged
merged 2 commits into from
Jul 5, 2021

Conversation

belgattitude
Copy link
Contributor

useTranslation() typings signature changed to allow a readonly version of namespaces argument...

Code

// From a readonly configuration. the const here is used
//  - to guarantee immutability (type level)
//  - helps typescript to narrow the type
//
export const nsConfig = ['ns1', 'ns2'] as const; 

// An example component
export const Test: React.FC = () => {
   const { t } = useTranslation(nsConfig);
   return <></>
}

Prior to this P/R, typecheck in strict mode would complain about

TS2345: Argument of type 'readonly ["common", "demo"]' is not assignable to parameter of type 'Namespace<"common" | "demo" | "home"> | undefined'.   The type 'readonly ["common", "demo"]' is 'readonly' and cannot be assigned to the mutable type '("common" | "demo" | "home")[]'.

Alternative hack:

It's possible to remove the readonly from readonly config, lacks elegance

export const nsConfig = ['ns1', 'ns2'] as const; // the `const` modifier will
type Writeable<T> = { -readonly [P in keyof T]: T[P] };
export const Test: React.FC = () => {
   const { t } = useTranslation(nsConfig as Writeable<typeof nsConfig>);
   return <></>
}

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • documentation is changed or added

@adrai adrai requested a review from pedrodurek July 5, 2021 15:20
@coveralls
Copy link

coveralls commented Jul 5, 2021

Coverage Status

Coverage remained the same at 96.583% when pulling a14279c on belgattitude:typings-allow-ro-ns into c0fb97f on i18next:master.

Copy link
Member

@pedrodurek pedrodurek left a comment

Choose a reason for hiding this comment

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

LGTM! 😄

@adrai adrai merged commit 62c5bc1 into i18next:master Jul 5, 2021
@adrai
Copy link
Member

adrai commented Jul 5, 2021

thx, included in v11.11.1

@belgattitude
Copy link
Contributor Author

@adrai Thanks for the speed :)

Unfortunate incompleteness in my P/R, hard to see but here's the continuation of it

feat(typings): support readonly namespaces in TFuncKey #1340

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants