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

Update serverSideTranslation args type #2097

Merged
merged 2 commits into from Mar 1, 2023

Conversation

mayone-du
Copy link
Contributor

Thanks for developing a great library!

The useTranslation function completes the namespace defined in CustomTypeOptions on i18next.d.ts, but the serverSideTranslations function does not, so I changed the type.

(If the change causes any inconvenience, close this PR.)

Checklist

  • only relevant code is changed (make a diff before you submit the PR)
  • run tests npm run test
  • tests are included
  • commit message and code follows the Developer's Certification of Origin

@mayone-du mayone-du changed the title Update SSRConfig ns type Update serverSideTranslation args type Feb 24, 2023
@adrai
Copy link
Member

adrai commented Feb 24, 2023

Why is that change necessary?
Is there an issue?

@mayone-du
Copy link
Contributor Author

mayone-du commented Feb 24, 2023

The reason is that the getServerSideTranslations argument is not type safe.
This is not in the issue, but I created a PR because I was unhappy with it while using it.

@adrai
Copy link
Member

adrai commented Feb 24, 2023

I'm not a TS expert, but I don't think Namespace[number] is correct, @pedrodurek @belgattitude?

@mayone-du
Copy link
Contributor Author

Since the original Namespace type definition is T | T[], Namespace[number] narrows it down to T.

(T contains namespace if it is defined, or a string if not.)

@adrai adrai merged commit 2c7f678 into i18next:master Mar 1, 2023
@adrai
Copy link
Member

adrai commented Mar 1, 2023

included in v13.2.0

@mayone-du mayone-du mentioned this pull request Mar 2, 2023
4 tasks
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.

None yet

2 participants