Update react-i18next types for 8.0.7 #29807

Jessidhia · 2018-10-17T09:42:41Z

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change. (Run with npm test.)
- npm test is failing on an unrelated package, some package.json lint?
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).
- some of the lints were there before

Select one of these and delete the others:

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: https://github.com/i18next/react-i18next/blob/master/CHANGELOG.md#800
Increase the version number in the header if appropriate.
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

Jessidhia · 2018-10-17T09:43:35Z

types/react-i18next/src/context.d.ts


 export interface ReactI18NextOptions {
    wait?: boolean;
    withRef?: boolean;
    bindI18n?: string;
    bindStore?: string;
+    /**
+     * @deprecated This option is now ignored and the function name is always `t`
+     */
    translateFuncName?: string;


I could change its type to 't' or just remove it altogether. I only kept it because it still technically exists in the return from getDefaults().

Jessidhia · 2018-10-17T09:45:46Z

types/react-i18next/src/context.d.ts

+
+export type ContextInferableComponentEnhancerWithProps =
+    <P extends { [key: string]: any }>(component: React.ComponentClass<P> | React.StatelessComponent<P>) =>
+        React.ComponentClass<Omit<P, keyof ReactI18nContext> & React.ClassAttributes<never>>;


I've been adding React.ClassAttributes<never> to the HOC to help avoid accidentally saving a ref to the HOC instead of the actually wrapped component. The getWrappedInstance() API is gone from react-i18next.

I also tried using React.ClassAttributes<React.ComponentClass> but that was not causing an expected type error on tests.

Jessidhia · 2018-10-17T09:46:40Z

types/react-i18next/src/withNamespaces.d.ts

+
+type ComponentPropsWithInnerRef<T extends React.ComponentType<any>> =
+    (T extends React.ComponentClass<infer P>
+        ? P & { innerRef?: React.Ref<InstanceType<T>> }


See i18next/react-i18next#534

This only works with withNamespaces for now.

Jessidhia · 2018-10-17T09:47:26Z

types/react-i18next/src/context.d.ts

+        : T extends React.SFC<infer P>
+            ? P
+            : {}
+    );


type ComponentProps<T extends React.ComponentType<any>> = T extends React.ComponentType<infer P> ? P : {}

sounds like something that could be useful in react.d.ts itself, at least until infer is allowed in places other than conditional type conditions.

The any is required by the way; the type argument is not contravariant so it would not allow a component to receive more props than are enumerated.

Jessidhia · 2018-10-17T10:05:47Z

Should I fix the lints on the v7 branch?

Also, what should be done about the helper types that are not exported (and were not exported to begin with); use export {} or export them too even though they're not library types?

I also kinda ended up abusing CI, but running npm test locally produces:

Error: In .../DefinitelyTyped/types/react-native-svg-charts/package.json: Dependency react-native-svg not in whitelist.
If you are depending on another `@types` package, do *not* add it to a `package.json`. Path mapping should make the import work.
If this is an external library that provides typings,  please make a pull request to types-publisher adding it to `dependenciesWhitelist.txt`.

Which specific package errors varies.

typescript-bot · 2018-10-17T10:22:29Z

@Kovensky Thank you for submitting this PR!

Because this is a new definition, a DefinitelyTyped maintainer will be reviewing this PR in the next few days once the Travis CI build passes.

In the meantime, if the build fails or a merge conflict occurs, I'll let you know. Have a nice day!

typescript-bot · 2018-10-17T10:22:30Z

@Kovensky The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

tylerexmg · 2018-10-17T11:19:16Z

types/react-i18next/src/withNamespaces.d.ts

+
+export interface WithNamespaces {
+    <TNamespace extends string>
+    (namespaces: TNamespace | TNamespace[] | undefined, options?: Omit<TranslateOptions, "translateFuncName">):


When testing this typing on a local project, Typescript was telling me that withNamespaces was expecting 1 or 2 arguments. Considering withNamespaces loads the defaultNS defined in i18next if nothing is passed through it, would it be better to make namespaces optional here instead of defining undefined?

Jessidhia · 2018-10-17T11:21:10Z

I intentionally made the first argument required because the intended API if you don't need any namespaces is `withI18n`.

…

On Wed, Oct 17, 2018 at 20:19 Tyler Pepper ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In types/react-i18next/src/withNamespaces.d.ts <#29807 (comment)> : > + +export type WrapperComponentClass<T extends React.ComponentType<any>> = + React.ComponentClass< + Omit<ComponentPropsWithInnerRef<T>, keyof InjectedProps> & + React.ClassAttributes<never> & + TranslateHocProps + >; + +// Injects props and removes them from the prop requirements. +// Adds the new properties t (or whatever the translation function is called) and i18n if needed. +export type InferableComponentEnhancerWithProps = + <T extends React.ComponentType<any>>(component: T) => WrapperComponentClass<T>; + +export interface WithNamespaces { + <TNamespace extends string> + (namespaces: TNamespace | TNamespace[] | undefined, options?: Omit<TranslateOptions, "translateFuncName">): When testing this typing on a local project, Typescript was telling me that withNamespaces was expecting 1 or 2 arguments. Considering withNamespaces loads the defaultNS defined in i18next if nothing is passed through it, would it be better to make namespaces optional here instead of defining undefined? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#29807 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEdfZcDFrMvurkQBsHdLXUwPq1NjhbWks5ulxJQgaJpZM4Xjhm-> .

These have been released upstream as of react-i18next 8.1.0. See i18next/react-i18next#557. Closes DefinitelyTyped#29408 Closes DefinitelyTyped#29480 Closes DefinitelyTyped#29685 Closes DefinitelyTyped#29807

The upstream library now ships it's own as of react-i18next 8.1.0. See i18next/react-i18next#557. Closes DefinitelyTyped#29408 Closes DefinitelyTyped#29480 Closes DefinitelyTyped#29685 Closes DefinitelyTyped#29807

beheh · 2018-10-22T13:13:14Z

This PR is probably obsolete, as typings have now been included upstream: i18next/react-i18next#557 and released in 8.1.0.

Jessidhia · 2018-10-23T01:08:47Z

Unfortunately the upstream typings are quite lacking compared to the typings here... anys or things that are effectively any where generics should be, for example.

Guess it'll need some patching.

beheh · 2018-10-23T09:16:27Z

Right - I think in that case we should just try to get these into upstream using PRs there. The maintainer there has shown interest in accepting changes fast and I'm already seeing a bunch of PRs on that repo.

typescript-bot · 2018-10-23T10:25:52Z

@Kovensky I haven't seen anything from you in a while and this PR currently has problems that prevent it from being merged. The PR will be closed tomorrow if there aren't new commits to fix the issues.

typescript-bot · 2018-10-24T10:26:25Z

@Kovensky To keep things tidy, we have to close PRs that aren't mergeable but don't have activity from their author. No worries, though - please open a new PR if you'd like to continue with this change. Thank you!

Jessidhia added 2 commits October 17, 2018 18:29

Update react-i18next types for 8.0.7

9a7c2ae

Improve react-i18next test

3a98651

Jessidhia commented Oct 17, 2018

View reviewed changes

Jessidhia added 2 commits October 17, 2018 18:52

react-i18next: Update the files in the tsconfig.json

1fe65b3

Correct tsconfig for react-i18next/v7

9b06e4b

typescript-bot added New Definition This PR creates a new definition package. The Travis CI build failed labels Oct 17, 2018

tylerexmg reviewed Oct 17, 2018

View reviewed changes

beheh mentioned this pull request Oct 22, 2018

[react-i18next] Remove typings as they were upstreamed #29945

Merged

9 tasks

typescript-bot added the Abandoned This PR had no activity for a long time, and is considered abandoned label Oct 24, 2018

typescript-bot closed this Oct 24, 2018

Jessidhia mentioned this pull request Oct 25, 2018

Version 8.1.0 is hard for previous TypeScript users to upgrade i18next/react-i18next#559

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update react-i18next types for 8.0.7 #29807

Update react-i18next types for 8.0.7 #29807

Jessidhia commented Oct 17, 2018

Jessidhia Oct 17, 2018

Jessidhia Oct 17, 2018

Jessidhia Oct 17, 2018

Jessidhia Oct 17, 2018

Jessidhia Oct 17, 2018

Jessidhia Oct 17, 2018

Jessidhia commented Oct 17, 2018 •

edited

Loading

typescript-bot commented Oct 17, 2018 •

edited

Loading

typescript-bot commented Oct 17, 2018 •

edited

Loading

tylerexmg Oct 17, 2018

Jessidhia commented Oct 17, 2018 via email

beheh commented Oct 22, 2018

Jessidhia commented Oct 23, 2018 •

edited

Loading

beheh commented Oct 23, 2018

typescript-bot commented Oct 23, 2018

typescript-bot commented Oct 24, 2018

Update react-i18next types for 8.0.7 #29807

Update react-i18next types for 8.0.7 #29807

Conversation

Jessidhia commented Oct 17, 2018

Jessidhia Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia commented Oct 17, 2018 • edited Loading

typescript-bot commented Oct 17, 2018 • edited Loading

typescript-bot commented Oct 17, 2018 • edited Loading

tylerexmg Oct 17, 2018

Choose a reason for hiding this comment

Jessidhia commented Oct 17, 2018 via email

beheh commented Oct 22, 2018

Jessidhia commented Oct 23, 2018 • edited Loading

beheh commented Oct 23, 2018

typescript-bot commented Oct 23, 2018

typescript-bot commented Oct 24, 2018

Jessidhia commented Oct 17, 2018 •

edited

Loading

typescript-bot commented Oct 17, 2018 •

edited

Loading

typescript-bot commented Oct 17, 2018 •

edited

Loading

Jessidhia commented Oct 23, 2018 •

edited

Loading