Flow: Cannot set defaultProps with React.memo and React.forwardRef

Created on 13 Feb 2019  路  24Comments  路  Source: facebook/flow

Flow version: 0.92.1 (flow-bin)
macOS 10.14.2

Expected behavior

You can set defaultProps on a component returned from React.memo and React.forwardRef.

Actual behavior

Flow will yield an error with the following message:

Cannot assign object literal to App.defaultProps because property defaultProps is missing in React.AbstractComponentStatics [1].


Example with React.forwardRef:

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBdkwBDAZzACUBTUgY2KnzizAHJ87H2BuVYMDDY8Rat0IARAPIBZMM1ZgARFwaEAtABNWy-qkIBPHDTAAFFjkoBeMAG9UYMADtSWGgC4w5QvgzOAcwAaVABffXo4Zx8wAEEcHDBbWnUAOigCBFJ8LVooAAp8nEtyLws4KwBKJIA+e0cwLkIAV3xnMAAeLQwANxqACxoYeCD7YoryVNd3UI7gbr6wyv14nFStGihSZphCcqsk+qdpzw5SZyjDLDhm8nYQ8PRBMEjo4hY4QgBRGBp3Z2Ith09Ga-0IqQCNG+vzBACFDABJLT5VRwT7KZYCIQpRgyWSpLjODb4fIdVZgYA1UYfaF-GgAzFAA

Corresponding codesandbox showing that setting defaultProps should work:

https://codesandbox.io/s/xv8m3qy6jo


Example with React.memo:

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBdkwBDAZzACUBTUgY2KnzizAHJ87H2BuVYMDDY8Rat0IARAPIBZMM1ZgARFwaEAtABNWy-qkIBPHDTAAFFjkoBeMAG9UYMADtSWGgC4w5QvgzOAcwAaVABffXo4Zx8wAEEcHDBbWnUAOncsOAAKLJxLci8LOCsASiSAPntHMC5CAFd8ZzAAHi0MADdygAsaGHgg+zzi8lTXd1Dm4DbOsJL9eJxUrRooUjqYQiKrJKqnMc8OUmcow0y68nYQ8PRBMEjo4hY4QgBRGBp3Z2JbHXo6z8IqQCNFe7wBACFDABJLRZVRwZ7KOYCIQpRgyWSpLjOZb4LLNBZgYDlAZPUEfGhfZFAA

There is a workaround with React.memo where you can set defaultProps on the function component and then afterwards export a memoized wrapper like: React.memo<React.Config<Props, typeof defaultProps>>(App); But I'm not sure if React.Config is intended to be used this way and you can't use this workaround for React.forwardRef.

enhancement

Most helpful comment

Default parameters are better, but why does a type-checker need to decide whether to allow the defaultProps parameter or not? React supports it, so Flow should support it. Typechecker's job should be to ensure types, not to judge whether a supported API is good or bad.

All 24 comments

defaultProps doesn't make much sense in function components. They were added to classes to share extended props across all class methods. Destructuring with defaults works less trickier in single function scope.

This works.

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBdkwBDAZzACUBTUgY2KnzizAHJ87H2BuVYMDDY8Rat0IARAPIBZMM1ZgARFwaEAtABNWy-qkIBPHDTAAFFjkoBeMAG9UYMADtSWGgH4AXGHKF8GM4A5gA0qAC++vRwzn5gAII4OGC2tOoAdFAECKT4WrRQABSFdq7uKRykzjGGWHAAruTs4T4WcFYAlCkAfPaOYFyE9fjOYAA8WhgAbt0AFjQw8CH2ZTThY8CTMxEd+oJg0bHELHCEAKIwNO7OxLY69PXXhOlBNOeXTwBChgCSWoWqOCnZS7ARCNKMGSydJcZxaGj4QpjRLJYDdZYnd5XGg3UFAA

https://flow.org/try/#0PQKgBAAgZgNg9gdzCYAoVBLAtgBzgJwBdkwBDAZzACUBTUgY2KnzizAHJ87H2BuVYMDDY8Rat0IARAPIBZMM1ZgARFwaEAtABNWy-qkIBPHDTAAFFjkoBeMAG9UYMADtSWGgH4AXGHKF8GM4A5gA0qAC++vRwzn5gAII4OGC2tOoAdO5YcAAUOXYubqa27KTOMYbZAK7k7OE+FnBWAJQpAHz2jmBchFX4zmAAPFoYAG5tABY0MPAh9jiW5Omu7uGDwCPjEc36gmDRscQscIQAojA07s7Etjr0VVeE6UE0ZxePAEKGAJJaOapwE7KHYCIRpRgyWTpLjOLQ0fA5QaJZLANpzY5vS40a4goA

We intentionally don't accept defaultProps on AbstractComponent. If we allowed it, we would need to propagate a third type variable to represent it. As @trysound said, there are other ways to get this desired functionality.

Feel free to reopen if you have any follow-up questions.

@jbrown215 @TrySound Makes sense. Thanks!

I don't think this issue is resolved.

I'm still having issues with React.memo, default props and flow. Here's a repro of the issue with defaultProps:

/* @flow */

import * as React from 'react';

type PropsType = {
  name: string,
};

const MyComponent = (props: PropsType) => {
  return <div>hello, {props.name}</div>
};

MyComponent.defaultProps = {
  name: 'default'
};

const MemoApp = React.memo<PropsType>(MyComponent);

// This works because the default props are considered
<MyComponent />;

// This doesn't work because the default props are NOT considered
<MemoApp />;

Run this code

Using function argument defaults (as per @TrySound) is not a good solution as the defaults for destructuring would require you to set the type as optional (ie. name?: string) instead of required (ie. name: string). My component requires this prop, so it shouldn't be optional. Having a default ensures this is defined correctly, it's just that the typing for React.memo doesn't seem to be smart enough to handle it.

@EvHaus It's actually a better solution since you don't need to jump across the code and look if prop is optional or not. You specify it's optionality explicitly in type. Defaults are implementation details of component body. defaultProps was always a hack around classes.

Default parameters are better, but why does a type-checker need to decide whether to allow the defaultProps parameter or not? React supports it, so Flow should support it. Typechecker's job should be to ensure types, not to judge whether a supported API is good or bad.

@satya164: We didn't make a value judgement about defaultProps. Supporting defaultProps on AbstractComponent comes at a huge expense and we decided it would not be worth the tradeoff.

defaultProps are supported by both function components and class components.

This issue isn't only with default props, it also applies to any other static property applied to the component.

For example, this isn't possible:

const Navigation = React.memo(props => /* ... */) // or any HOC that returns React.AbstractComponent

Navigation.Item = NavigationItem

/ ... /

<Navigation>
  {items.map(item => <Navigation.Item {...item} />)}
</Navigation>

Now, one could argue whether the navigation item component should be put in the statics of the navigation component. That decision should be left to the API designer though. It would be nice if Flow supported this.

EDIT: Another extremely common use case is adding GraphQL fragments statically to the component.

@jbrown215 Is it not worth?? Okay: so what are you recomendating? Use destruction inside like React.memo((props) => Checkbox({...defaultProps, ...props})); or what? You mean that i have to write shitcode because defaultProps? Program is going well, tests are going well but flow show me errors. So what are you really recomendating for us: do not use flow inside memo functional components? Can i ask you about please: why did you decide that it's not worth?

Why is this closed?

This is not something we are going to change.

I recommend using destructuring with defaults instead.

Alternatively, you can set the defaultProps and suppress the error if you need to add defaultProps to an AbstractComponent.

Is there a reason you need to set the defaultProps _after_ the component is wrapped instead of before?

@jbrown215 it can be component from React.forwardRef which styled-components is using, this means it is impossible to set defaultProps on styled-components

https://github.com/flow-typed/flow-typed/pull/3228#issuecomment-513745964

import styled from 'styled-components'

type Props = {
   color: number
}

const Foo = styled('div')`
  color: ${(props: Props) => props.color} 
`
// error
Foo.defaultProps = {
  color: '#000'
}

At least consider making it mixed, or Partial<Config> (not broken $Shape I guess)

At least consider making it mixed, or Partial<Config> (not broken $Shape I guess)

Hi @goodmind, is Partial<> going to be a new utility type?

@omninonsense No, but I hope that someday, $Shape actually works too for this purpose
The problem is that Config is contravariant generic in AbstractComponent so it errors
image

I'm convinced that this isn't needed because React wants to deprecate defaultProps anyway

(specifically on function components, which would make it unsound to use on AbstractComponent)

I use this in TS

export function useDefaultProps<P extends object, DP extends Partial<P>>(
  props: P,
  defaultProps: DP
): DP & P {
  return {
    ...defaultProps,
    ...props
  };
}

interface MyComponentProps {
  x: number;
  y?: number;
}

const MyComponent = memo((_props: MyComponentProps) => {
  const props = useDefaultProps(_props, {
    y: 10
  })
  // or
  const {x, y} = useDefaultProps(_props, {
    y: 10
  })

  // use props as usual
})

Basically need to use @xaviergonz solution because destructuring props comes with its own downsides. For example, I have a hook that requires me to pass in the props object. The defaults are destructured variables... so can be a bit of conflict depending on what you're doing. Plus null vs undefined in regard to defaults is mildly confusing (I get why, but it's very easy to let this slip). Though I'm glad to hear defaultProps is going away because I've seen some ugly stuff.

@goodmind I don't think this issue should be closed unless the react team is also considering to deprecate prop types as well.

both defaultProps and propTypes go hand in hand with each other.

for example, if you have a boolean value called disabled and you mark it required in propTypes, if you specify a value for disabled in defaultProps, it meets the requirement, however if you do the same with a destructured value of props, it does not meet the requirements even though people are saying that they're basically "functionally equivalent".

So we changed our code to use defaultProps because Flow was not able to understand that destructuring with defaults makes a prop not undefined and now, after changing our entire codebase to meet this, you are telling us that I have to change this to the opposite? This is not something I want to do, and I don't want to have an inconsistent codebase. If this is a valid react patter I don't understand why flow forces us to something which is worse.

So we changed our code to use defaultProps because Flow was not able to understand that destructuring with defaults makes a prop not undefined

Can you show an example of this?

Sorry,an example of exactly what? That you need to specifically tell flow a react prop is optional even if you provide a default value for it? Sure thing:

https://flow.org/try/#0JYWwDg9gTgLgBAJQKYEMDG8BmUIjgcilQ3wChy0IA7AZ3gCEAbFOAXjgAoBvFAGgCNWmFIxpIAvgC4ekuPwgRGvObPmLxASlYA+LuThwiMAK5QqcFgDJLcuNbgAiABJJGjCAEIHpceVIAeJhYUVi4YKGMJAHptIA

That's not a bug. Flow does infer it to be optional (try removing the annotation) but you are explicitly annotating it to be non-optional. Flow will always use your annotations. If you give it an annotation with an optional property it'll retain its non-annotated behavior.

Except, if you provide a default value on the defaultProps, which is what flow is not capable of handle and it's what this issue is all about. The example I provided was just how flow behaves in absence of defaultProps.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ghost picture ghost  路  3Comments

l2silver picture l2silver  路  3Comments

funtaps picture funtaps  路  3Comments

glenjamin picture glenjamin  路  3Comments

cubika picture cubika  路  3Comments