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

Fix typings for inferno-redux Provider component #1495

Merged
merged 5 commits into from
Nov 8, 2019

Conversation

Seally
Copy link
Contributor

@Seally Seally commented Nov 6, 2019

Objective

This PR fixes some typing issues encountered when using inferno and inferno-redux in TypeScript environments.

The first fix fixes a typing issue that occurs when the root reducer function's second argument is not AnyAction. Previously Provider will reject any Redux Store<S, A> when A is not AnyAction unless it has been type casted as such. This fix was mostly copied from how @types/react-redux handles it.

The second fix fixes a type intersection issue caused by this interface

export interface Props {
store: Store<any>;
children?: VNode | null | undefined;
}

being passed here (aliased as P):
public props: {
children?: InfernoNode;
} & P;

This leads to the type of props.children to be:

VNode | (string & VNode) | (number & VNode) | (false & VNode) | (true & VNode) | (InfernoElement<any> & VNode) | (import("../../../inferno/src").InfernoNodeArray & VNode) | (JSX.Element & VNode) | null | undefined

Types like string & VNode and true & VNode is likely incorrect. I have been unable to determine why VNode was given as the type for Props['children'], but it seems to be an error and I've changed this to InfernoNode.

The last fix manually specifies the type of Provider#render() as a workaround for #1460. Without that line, TypeScript may infer the type and expand the InfernoNode type alias during .d.ts generation, leading to an incorrect in-place import for InfernoNodeArray to be generated.

Closes Issue

It closes Issue #1460.

@coveralls
Copy link
Collaborator

coveralls commented Nov 6, 2019

Coverage Status

Coverage remained the same at 95.059% when pulling cdc0382 on Seally:provider-type-fix into deccbe5 on infernojs:master.

@Havunen
Copy link
Member

Havunen commented Nov 8, 2019

Hi @Seally

Changes looks good to me. Can you add some way to test the issue this fixes please :-)

It could be done for example by creating tsx test file to render something

@Havunen
Copy link
Member

Havunen commented Nov 8, 2019

Thank you @Seally

@Havunen Havunen merged commit 0d98044 into infernojs:master Nov 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants