Skip to content

Provider API breaks strict typing #290

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

Closed
mohsen1 opened this issue Feb 15, 2016 · 13 comments
Closed

Provider API breaks strict typing #290

mohsen1 opened this issue Feb 15, 2016 · 13 comments

Comments

@mohsen1
Copy link

mohsen1 commented Feb 15, 2016

The Provider API magically provides props for a component that requires those props. For example if we have a regular React code before provider is used like this:

const todos = [];
render(<App todos={todos} />, document.body)

After using Provider we change it to following:

render(<Provider store={store}><App /></Provider>, document.body)

this is not a problem in regular JS or JSX, but when I use TypeScript, the compiler complains that App requires required prop todos. Of course I can make it an optional property but that's not optimal.

@gaearon
Copy link
Contributor

gaearon commented Feb 15, 2016

<Provider> itself has no effect on what props a component receives.

You probably confuse it with connect(). This is what injects those props into your component.
It is explicit:

import { connect } from 'react-redux'

class App extends Component { }

export default connect(
  mapStateToProps
)(App)

I don’t know enough about TypeScript but I imagine you could write bindings for connect() which would tell TypeScript how it works. Unfortunately I don’t think we can do anything on our side to better support this. If you look at connect() source, you will see that it passes those props explicitly to the component you wrap with connect() so there is no magic here.

@gaearon gaearon closed this as completed Feb 15, 2016
@mohsen1
Copy link
Author

mohsen1 commented Feb 15, 2016

Thanks for clarification. I use TypeScript type definitions from TSD repository. It works great with JSX.

Consider following example:

export default class App extends Component<AppProps, AppState> { render(){/*...*/} }

which AppProps and AppState are simply interfaces. If AppProps is this interface:

interface AppProps {
  todo: Array<{}>
}

This TSX code will complain that App needs required property todo:

import {render} from 'react';
import App from './app';

render(<App />, document.body);

Now if I use redux connect method in app.tsx like following:

import {connect} from 'react-redux';

class App extends Component<AppProps, AppState> { render(){/*...*/} }

export default connect(state=> {todos: state.todos})(App)

The TSX compiler will not understand that connect made a new component that does not have the same props.

This is most likely due to type definition or TSX capabilities. I just wanted to leave this write up for future references.

I'll post here if I could figure it out although I'm pretty new to React, Redux and TypeScript!

@gaearon
Copy link
Contributor

gaearon commented Feb 16, 2016

The problem is likely that those typings don’t tell TypeScript that parent props are going to be merged with what you return from mapStateToProps and mapDispatchToProps. We don’t maintain those typings officially so I can’t vouch for their quality.

Related: reduxjs/redux#1401.

cc @aikoven @ulfryk

@aikoven
Copy link

aikoven commented Feb 16, 2016

@mohsen1 I'm using custom typings for connect:

  export interface ClassDecorator<P, O> {
    (component: StatelessComponent<P>): ComponentClass<O>;
  }

  export function connect<O, S, D>(
    mapStateToProps?: MapStateToProps<O, S>,
    mapDispatchToProps?: MapDispatchToPropsFunction<O, D>|D
  ): ClassDecorator<O & S & D, O>;

  export function connect<O, S, D, P>(
    mapStateToProps: MapStateToProps<O, S>,
    mapDispatchToProps: MapDispatchToPropsFunction<D, O>|D,
    mergeProps: MergeProps<S, D, O, P>,
    options?: Options): ClassDecorator<P, O>;

  export interface MapStateToProps<O, S> {
    (state: any, ownProps?: O): S;
  }

  export interface MapDispatchToPropsFunction<O, D> {
    (dispatch: Dispatch, ownProps?: O): D;
  }

  export interface MergeProps<S, D, O, P> {
    (stateProps: S, dispatchProps: D, ownProps: O): P;
  }

They only work with stateless functional components and don't cover all cases, but types are inferred correctly:

const App = connect(
  selector,
  {
    actionCreator,
  }
)(props => {
    // `props` type is inferred from return type of `selector` 
    // and shape of action creators object
})

The major drawback that kept me from submitting these to DefinitelyTyped is that TypeScript doesn't understand decorators that change type.

Before that I had to explicitly specify props type and make every prop optional.

@mikekidder
Copy link
Contributor

@aikoven how is Angular2 solving it with their decorators?

@aikoven
Copy link

aikoven commented Feb 17, 2016

@mikekidder I'm not aware of it, could you please point to some examples?

@mohsen1
Copy link
Author

mohsen1 commented Feb 17, 2016

@aikoven can you share your full react-redux.d.ts file so I can try it out?

@aikoven
Copy link

aikoven commented Feb 17, 2016

@ulfryk
Copy link

ulfryk commented Feb 17, 2016

@gaearon - I also had this problem, but did not found any good solution. It's actually problem of how .tsx understands component class.

@mikekidder - Angular2 decorators are not useful here. Their way is binding a property of a class to input attr. If something similar was made for react it would look like this:

@connect
class App extends Component<{},{}> { 

    @connectProp('title')
    appTitle: string;

    render() {
        return <h1>{this.appTitle}</h2>
    } 

}

and now render(…, <App/>) would compile, because binding of title="…" property to component attribute appTitle is done under the hood and .tsx compiler thinks that App does not have any props. And this of course will work well if we omit using of regular props

@codeandcats
Copy link

Don't suppose this has been resolved nicely yet for decorating React components defined as classes?

@aikoven
Copy link

aikoven commented Sep 12, 2016

@codeandcats Nope, but there are some issues in TS repo to track, e.g.: microsoft/TypeScript#4881

@Zalastax
Copy link

I tried out #433 and those typings work as expected for me. The return type of connect is

interface ComponentDecorator<TOriginalProps, TOwnProps> {
    (component: ComponentClass<TOriginalProps>|StatelessComponent<TOriginalProps>): ComponentClass<TOwnProps>;
}

which is exactly what we want!

@codeandcats
Copy link

@Zalastax it looks like that PR has been closed. 😢 I don't suppose you feel like forking and re-requesting it? 🌹

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

No branches or pull requests

7 participants