Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Best practices for handling props connected to redux vs not connected to redux #327

Closed
Spiralis opened this issue Sep 21, 2016 · 9 comments

Comments

@Spiralis
Copy link

First of all; I wish there was someone who could write a blog entry/tutorial somewhere on the typescript redux matter. Especially now that redux itself has included typescript definitions, and since there has been a lot of discussing in the redux project about the best practices for this. I see that this project uses and extension, redux-typed, and I am not sure if these are all connected in regards to being a "good practice" or not.

I will use this post to explain my approach, as well as my actual problem, so that someone maybe can guide me to a better way of doing this.

When I create a new component I usually create it small, minimal. As I try to make them pure and dumb one of the immediate questions is which properties it should take. I then have a test-layout (or anywhere) that I use to inspect it and manually test it during development. A part of this process is deciding which props are required and not, trying to make sure that the component is well spec'ed.

In a typescripted environment, like for the projects created by this tool, this means setting the component up like this:

import * as React from 'react';
import { provide } from 'redux-typed';
import { ApplicationState } from '../store'; // The app store
import { actionCreators } from './store'; // This component's own store

interface MyComponentProps extends React.Props<any>{
    propBool: boolean;
    //...
}
export default class MyComponent extends React.Component<MyComponentProps, void> {
    //...
}

BTW: I am bundling files with functionality for the same component in component-folders, like this:

Components
|- MyComponent
|  |- index.tsx (holds the react component for MyComponent)
|  |- store.ts (holds the state, actioncreators and reducer for MyComponent)
|  |- styles.css (I am using CSS Modules, and this holds the styles for it, not shown in the code above)
|  OtherComponent
...
- boot-client.tsx
- boot-server.tsx
- configureStore.ts
- routes.tsx
- store.ts
...

This makes it easier to reuse components. Adding on to a project is drag-dropping it in and wiring state and reducers in the app store.ts. Literally 2 lines of code. This has been great!

Ok, next step si to set up more properties. When adding methods I usually add them as

propMethod: Function;

... which obviously has no additional type-cehcking. Credited due to the fact that I am a typescript newbie... But, it works for my testing purposes.

Later I usually connect it to the redux store (if needed - of course). Doing that means that after the component class we add some extra code:

const provider = provide(
    (state: ApplicationState) => state.myState,
    actionCreators
);
export default provider.connect(MyComponent);

At this time I have to use the provider.allProps as a parameter for the component props, and move the export default - otherwise it doesn't compile. Which also means that my test-properties are now no longer needed and can be deleted. That is fine for the scenarios where all the props are from the store. For other scenarios I found that I needed to use the withExternalProps approach. Also, I found that using allProps alone missed out on some of the props provided by React.Props<any>, like i.e. children.

So, even without extra props beyond the store and actioncreators, I usually do this:

const provider = provide(
    (state: ApplicationState) => state.myState,
    actionCreators
).withExternalProps<React.Props<any>>();

Which seems to do the trick.

Ok, and then to the meat of this:
I am finding it "messy" with property-definitions, compared to javascript React.PropTypes, getting this to work. I do appreciate the type-checking, but to what cost. I have been training people on the team on this and they get a bit confused when we get into this area.

Also, after having wrapped the component, I am finding that there is no way to control properties from the outside. Which kinda makes sense, but it is sometimes nice to be able to override one of the store props. Also, I haven't yet included reselect into the mix, and I am not sure that I dare doing it.

So, am I doing things the wrong way? Are there better ways? Or, are there any docs/tutorials or similar out there that could help me understand this better (if my understanding of this seems to be the issue)? I know I need more typescript training, btw, and working on it :).

@Spiralis Spiralis changed the title How to handle props for my components when they are connected to redux vs not connected Best practices for handling props connected to redux vs not connected to redux Sep 21, 2016
@SteveSandersonMS
Copy link
Member

Thanks for posting. I haven't closely followed changes in the Redux community lately so I'm not able to compare any patterns there with what we have in this template. If someone else has insight and wants to weigh in please do so!

When I get chance I will update the ReactReduxSpa template to take advantage of whatever has changed in Redux world lately, though it may be a while before I get to that.

@MarkPieszak
Copy link
Contributor

MarkPieszak commented Sep 21, 2016

@SteveSandersonMS Part of those changes are actually part of the problems with updating that template to TypeScript2, the @types are all based on the new concepts so things like compose() and Redux.Store all changed definition signatures etc. Just a heads up!

@Spiralis
Copy link
Author

Ah. I see. Thanks. I do like the advantages of typescript, but - at the same time I am feeling the pain of that method not being as mainstream as all samples and definitions out there. AFAICT, typescript is superior, but hoping for that to become the de-facto standard is perhaps wishing for too much... I haven't really tried flow as an alternative to typescript, so not sure if that would feel better. I picked up typescript just because this awesome project (thanks guys 👍) happened to use it as a part of it's setup. Without this project I'd still be in pure JS world I guess. And even though ES6+ is making that world a better place, it still is not as nice as typescript. I am starting to get used to it, but like I said; still some friction... I will keep hanging in here though :)

@MarkPieszak
Copy link
Contributor

It definitely takes a little bit of time to get used to typescript, but keep with it you'll be happy you did, it's incredible. It's slowly inching towards just becoming C# :) the same Crestor actually.

Flow is nice, but just does mor e code analysis, which is helpful, but you lose all the tooling and amazing news that is typescript . I can imagine someday they might even add that functionality to typescript someday... At least that's the vibe I got when I spoke with a few people on the TS team.

Either way like you said, at least we can finally stop coding with es5!!

@Spiralis
Copy link
Author

Related to my above question. How can I select a part of the state base on a react prop?

const provider = provide(
    (state: ApplicationState) => state.cards, // Select which part of global state maps to this component
    CardState.actionCreators                 // Select which action creators should be exposed to this component
);

With the above, where could I for instance say that I want to select only the card that have an id that matches the component prop.id, given that the state.cards contains i.e.:

state.cards = [
  {id:"1", data:""},
  {id:"test", data:"Test-card"}
] ;

I'd have hoped that the provide method also could pass along the incoming props, in order for the component to depend on only what it needs. All the samples I have found on this in general refers to mapStateToProps, but with redux-typed we use `connect() instead and I can't find any way to pass in the props to it.

This kinda brings up a related subject. How can I use reselect in combination with the connect() method?

@Spiralis
Copy link
Author

To answer my own question:

It seems that the provide() method has a lacking typedefinition. It only accepts state as an argument in the the function added as first param, even though the underlying connect is passing props too.

I changed the last line in StrongProvider.d.ts from:

export declare function provide<TOwnProps, TActions>(stateToProps: (appState: any) => TOwnProps, actionCreators: TActions): ComponentBuilder<TOwnProps, TActions, {}>;

to:

export declare function provide<TOwnProps, TActions>(stateToProps: (appState: any, props?:any) => TOwnProps, actionCreators: TActions): ComponentBuilder<TOwnProps, TActions, {}>;

Then I could access props from within the mapStateToProps function:

const provider = provide(
    (state: ApplicationState, props) => state.cards[props.id], // Select which part of global state maps to this component
    CardState.actionCreators                 // Select which action creators should be exposed to this component
);

@SteveSandersonMS, I don't know where the repo for redux-typed is, so I couldn't submit a patch for it myself.

@Spiralis
Copy link
Author

Spiralis commented Sep 27, 2016

And - I am guessing that props should be made typed too, but I am too much of a newbie in typescript to take on that task. I mean, I can guess that it might be able to use TOwnProps - or TExternalProps, or a combination? Or allow the user to pass the type in via provide<,,>, but I am not sure about how to use those. So, someone else might be better off tuning this I guess...

@empz
Copy link

empz commented Oct 6, 2016

@SteveSandersonMS Now that redux includes its own typings. Do we really need redux-typed?

It seems that package has been created only for the ReactRedux template and nobody else in the world is using it. I'm pretty sure you've said several times that you want to keep the "extra/non-standard" stuff to a minimum so people can learn the tools/framework as close to native as possible.

Might be a good idea to make use of standard redux code. Just throwing an idea.

@SteveSandersonMS
Copy link
Member

This discussion has largely become about the use or removal of redux-typed, so I'll close this in favour of a new issue specifically about that: #387

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

No branches or pull requests

4 participants