docs: annotating connected components with Flow

[wgao19]

What does this PR do?

Addressing #1001, add docs for annotating connected components with Flow

Summary of Changes

• Original doc for annotating connected components with Flow
• Duplicate the doc for versioned docs starting from 5.x
• Edit the version sidebar files for the doc to show up on every versions

Anything to take note

@markerikson I think this doc is still very drafty to be used as an official doc. Could you please provide some suggestions on the writing? I'd like to work on it to make it more readable.

It's better if we can have someone on the Flow team to review the content as well.

[netlify]

Deploy preview for react-redux-docs ready!

Built with commit 7900484

https://deploy-preview-1336--react-redux-docs.netlify.com

[jbrown215]

This looks good. I can't give specific feedback on the connect libdef because I'm far from a redux expert. @villesau may be able to provide more feedback there.

Thanks for writing this up!

[villesau]

Great initiative!

What caught my eye was the mix between exact & inexact props which you should not do as the result might be unexpected. In docs I would mostly use just exact objects & spreading. Then inexact & $Diff cases could be mentioned as a special cases.

[wgao19]

Hi @villesau thanks for the tip! I'll change everything to exact in this doc. Do you have a pointer to where I can learn more about the inexact $Diff cases? I only know it's tricky but the whys and hows are still a bit blurry to me.

[wgao19]

Hey @villesau @jbrown215 I've updated the content addressing the discussions above. Could you please review again?

Summary of changes:

• Begin with an added section for annotation at function return, this shall be the simplest way of annotating connect, and does not require the libdef.
• Revised libdef intro to be more friendly
• Added notes on recommended use of exact objects for props
• Removed the chunk with $Diff

Take note that the two markdown files have the same content. This is to serve versioned docs. You don't have to read both. Here is the link to a preview of what this doc will look like when it lands.

[villesau]

Type casting should not be needed. In overall type casting should be avoided IMO. You can probably do something like this:

export default connect<Props, OwnProps, _, _, _>(
  mapStateToProps,
  mapDispatchToProps
)(MyComponent)

In what kind of cases would you prefer casting over type parameters? If there are valid use cases, there might be room for improvement in the libdefs as well.

[wgao19]

In what kind of cases would you prefer casting over type parameters? If there are valid use cases, there might be room for improvement in the libdefs as well.

In our code base we have a lot of nested higher order components alongside connect. So instead of doing this:

import { compose } from "redux";
type AllProps = {/** ... */};
type withAProps = {/** ... */};
// and so forth
compose(
  connect(mapState, mapDispatch), // T_T
  withA<$Diff<$Diff<$Diff<AllProps, withDProps>, withCProps>, withBProps>>,
  withB<$Diff<$Diff<AllProps, withDProps>, withCProps>>,
  withC<$Diff<AllProps, withDProps>>,
  withD<AllProps>
)(ComponentWithEverything);

We figured we would do this instead:

type OwnProps = {/** props essential to this component */};
type AllProps = { // spreaded props should be exact
  ...OwnProps, ...withAProps, ...withBProps, ...withCProps, ...withDProps,
}
const ComponentWithEverything = (props: AllProps) => {/** ... */};
export default (
  compose(withA, withB, withC, withD)
    (ComponentWithEverything): 
      React.AbstractComponent<OwnProps>
);

I previously included the description to this situation, but removed it since $Diff is tricky. And I put the casting up front because it requires less set up. I am not 100% sure about the best practices though, any suggestions?

[villesau]

@jbrown215 what do you think about above? Casting feels quite nasty to me, but the example above looks almost impossible to type. Redux compose is an alias to Flows internal compose

[wgao19]

What's the downside of casting?

I realize that within each of the hocs, providing explicit type parameters helps the hocs to deduce their own return component types, but we're only feeding them to the next layer. So it seems to me that it's not as necessary since all of those calls stay within one file. And given it's not practically possible as we have some big hundreds of components like that 😅, the workaround we figured out was to annotate at function call return.

[villesau]

the workaround we figured out was to annotate at function call return.

This is the point. Should workarounds be documented as a recommended way of working? Maybe if it's explicitly explained that it's a workaround.

There is a risk that you introduce some unintended discrepancies. see e.g these examples:
https://flow.org/try/#0BTDeEMC4AIEYCYDMBfGoBGB+GA7ArgLboCmATsgJRpTT5FmUDcAUM2DQimlroSeRRZtgFaAF4AfNFCpowGnX6jJ0AG4B7AJYATQUA

Also, it's not very "flow" way of writing types. Maybe compose needs an ability to take type parameters in. This way Flow could maybe infer the rest.

[jbrown215]

I actually am a big fan of casting. It has no runtime effect, so it really is just asking flow to ensure that the value matches the type. An equivalent way of doing this without casting is just:

// Before
module.exports = (SomeExpr: SomeType);

// After
const exports: SomeType = SomeExpr;
module.exports = exports;

IMO, you should just use whichever syntax is easier to express your intent. Sometimes that's explicit type args, other times that's a cast.

[wgao19]

@villesau I’m trying to understand this example you linked — is it casting the object twice and therefore producing the discrepancies?

[villesau]

It's just an example where you can cast the object to something else, but can't actually cast it back to original type. This is totally by design: Inexact objects allows that. But it also makes it possible to make mistakes like in the example. The point is that you might loose some type information with casting if you are not careful.

[wgao19]

So.. We have notes about inexact objects being tricky, I guess I shall stress more of that?

And looks like we're not triggering the tricky serial casting thing since the two casting happen on different objects? The first one on annotating the unconnected component, the second time the exported connected component.

Meanwhile, I feel the benefit of annotating nested hocs + connect this way is larger than the potential caveat we're facing. Because we don't have hold of how the other hocs pass types information around. And so I feel annotating once at export is an expressive clean cut, after all simplicity is also helpful to reduce mistakes. We've been doing this with our code base and Flow is able to cover those connected components correctly.

[wgao19]

Hey @markerikson would you like to take a look at this piece now?

[markerikson]

I'm mostly away atm, so I don't have time to go through this in detail.

I think my main observation atm is that the title and the content seem just a bit too specific for a top-level page. Conceptually, I'd like to see either a single "Static Typing" docs page that covers both general TS and Flow usage, or perhaps a "Static Typing" sub-category within "Usage" that then has separate pages for TS and Flow, with the seemingly more-specific content here being part of the Flow page.

[wgao19]

Hey @markerikson looks like the sidebar does not support nested categories. What about I put the content about Flow into a "Static Typing" page, and leave out the TypeScript section and mention that we're looking for PR? I'm not practically experienced on the TS usage so I'll have to leave it out for now.

[markerikson]

Hmm. That's odd, because we've specifically got a nested category in the core Redux docs - the "Recipes > Structuring Reducers" section:

https://redux.js.org/recipes/structuring-reducers/structuring-reducers

Setup:

https://github.com/reduxjs/redux/blob/ff8fce055b79c63e085c4e1d394bf637523626b7/website/sidebars.json#L46-L63

Do we need to bump the Docusaurus version for React-Redux or something?

[wgao19]

Hey @markerikson I created the subcategory, would you take a look at how it looks now?

[timdorr]

We're not Flow experts here and don't have the expertise to know if this is correct, so I'm going to close this out.

Until we adopt TS for this library, let's leave out static typing docs. We don't control the types, so we shouldn't have to document them (docs could easily go out of sync).

[markerikson]

@timdorr : I'll disagree with that one. We've already got a "Static Typing" docs page over in the Redux core that lists the basic approaches, and that's what I'm going for over in #1439. I particularly want to officially document the ConnectedProps<T> approach publicly.

[timdorr]

But we don't maintain those types. If the folks on DT want to change that interface, we have no control over that and then the docs are out of sync.

[markerikson]

We haven't entirely maintained the Redux core types either.

Given the prevalence of TS usage, and that we recommend using TS, I want to officially document the basics of TS usage.