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

React-navigation: fixed flow type error with React$ #2522

Merged
merged 1 commit into from Nov 21, 2018

Conversation

flbaue
Copy link
Contributor

@flbaue flbaue commented Jul 20, 2018

I just created a new react native project (0.56) and added react-navigation (2.7.0) and flow libdef react-navigation_v2.x.x.
As a result, I get a type error when I try to use the result of createDrawerNavigator (or the others).

yarn flow status
yarn run v1.7.0
$ /Users/.../.../.../App/node_modules/.bin/flow status
Error ┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈┈ src/app.js:18:13

Cannot instantiate React.Element because in type argument ElementType:
 • Either a callable signature is missing in statics of React.Component [1] but exists in
   React.StatelessFunctionalComponent [2].
 • Or a callable signature is missing in withRouter [3] but exists in
   React.StatelessFunctionalComponent [2].
 • Or a callable signature is missing in withOptionalNavigationOptions [4] but exists in
   React.StatelessFunctionalComponent [2].
 • Or React.StatelessFunctionalComponent [1] is incompatible with statics of React.Component [5].
 • Or withRouter [3] is incompatible with statics of React.Component [5].
 • Or withOptionalNavigationOptions [4] is incompatible with statics of React.Component [5].

     src/app.js
      15│ type Props = {}
      16│ export default class App extends Component<Props> {
      17│   render() {
      18│     return <Navigator />
      19│   }
      20│ }
      21│

     flow-typed/npm/react-navigation_v2.x.x.js
 [1] 571│   > = React$ComponentType<{
     572│     ...Props,
     573│     ...NavigationContainerProps<State, Options>
     574│   }> &
 [3] 575│     withRouter<State, Options> &
 [4] 576│     withOptionalNavigationOptions<Options>

     /private/tmp/flow/flowlib_34616c66/react.js
 [2] 160│   | React$StatelessFunctionalComponent<any>
 [5] 161│   | Class<React$Component<any, any>>;



Found 1 error

Only showing the most relevant union/intersection branches.
To see all branches, re-run Flow with --show-all-branches
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

After some researching, I replaced the React$ inside the libdef with React. plus import * as React from "react". This is fixing the type error for me.

Is this the right solution for this? Or are there any better ways?

@flbaue flbaue changed the title Fixed flow type error with React$ React-navigation: fixed flow type error with React$ Jul 20, 2018
@sibelius
Copy link
Contributor

cc @Ashoat

@Ashoat
Copy link
Contributor

Ashoat commented Jul 20, 2018

Some context: @flbaue initially reported this issue in the react-navigation repo, and proposed the same fix. I wasn't sure how to proceed, since it appears to fix his issue, but importing like this is discouraging by the CONTRIBUTING docs.

@sibelius, do you know if any of this has changed? @flbaue, is there anything non-standard about your environment that may be affecting things here?

@sibelius
Copy link
Contributor

I think import does not work yet

@Ashoat
Copy link
Contributor

Ashoat commented Jul 20, 2018

Hmm, but the docs page I linked above says:

You can import types built into Flow (e.g. from react), only if you put the import statement inside the module declaration

@Ashoat
Copy link
Contributor

Ashoat commented Jul 20, 2018

What's confusing me is that the React.* types should be the same as the React$* types.

After reading a bit and remembering that import statements outside a declare module block get any-typed, I think I have a theory. This PR gets rid of the error @flbaue was seeing because it unintentionally any-types the imports.

@flbaue, if you move the import statement to inside the declare module, do the errors reoccur?

@flbaue
Copy link
Contributor Author

flbaue commented Jul 20, 2018

@Ashoat If I move the import inside, the error does reoccur. So you might be right.

Also I just noticed something. I am able to reproduce the type error in any project, when I render the navigator as a child of a react-redux provider. If the react-redux typedef is installed, I have the type error. If I render the Navigator as a child to a normal, self written, component, I don't have the error.
Also if I remove the react-redux typedef, the error goes away as well.

I created an example here: https://github.com/flbaue/TestProject

Copy link
Contributor

@Ashoat Ashoat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. My first thought was that the issue must be with react-redux then, but that libdef seems to any-type Provider's children, so I'm not sure why there would be any error...

Either way, given that this PR would any-type every use of React in the libdef, we definitely need some changes before we merge it.

@flbaue
Copy link
Contributor Author

flbaue commented Jul 20, 2018

@Ashoat Could you confirm the issue with my example? Just to make sure it's not just something weird on my side.

@Ashoat
Copy link
Contributor

Ashoat commented Jul 20, 2018

This is certainly strange. If I copy the type definition of Provider from react-redux's libdef into your App.js file, the error goes away. (I also needed to copy the type import of Store from redux, and remove the import of Provider.)

I wonder if it has to do with how react-redux's libdef is importing types from redux.

@flbaue
Copy link
Contributor Author

flbaue commented Jul 20, 2018

I found another way to fix the type issue.
When I replace React$ComponentType in line 568 with React$StatelessFunctionalComponent it also seems to work fine.

@flbaue flbaue reopened this Jul 20, 2018
@ujwal-setlur
Copy link

Using React$StatelessFunctionalComponent worked for me as well

@Ashoat
Copy link
Contributor

Ashoat commented Nov 14, 2018

I am seeing this issue now after updating the libdef for React Navigation 3.0. I'm not sure what's triggering it, but this patch allows us to circumvent the issue, and I don't have a better idea. Let's get this merged.

Ashoat added a commit to Ashoat/react-navigation that referenced this pull request Nov 14, 2018
1. `navigationOptions` in `RouteConfig`s is now `defaultNavigationOptions`
2. `create*Navigator` no longer return a container, they return a navigator
3. Introducing `createAppContainer`

Closes react-navigation#4722 by including flow-typed/flow-typed#2522
brentvatne pushed a commit to react-navigation/react-navigation that referenced this pull request Nov 15, 2018
1. `navigationOptions` in `RouteConfig`s is now `defaultNavigationOptions`
2. `create*Navigator` no longer return a container, they return a navigator
3. Introducing `createAppContainer`

Closes #4722 by including flow-typed/flow-typed#2522
@AndrewSouthpaw AndrewSouthpaw merged commit a04a342 into flow-typed:master Nov 21, 2018
@AndrewSouthpaw
Copy link
Contributor

Done! Thanks everyone.

ManuelKu1223 pushed a commit to ManuelKu1223/react-navigation that referenced this pull request Jun 5, 2019
1. `navigationOptions` in `RouteConfig`s is now `defaultNavigationOptions`
2. `create*Navigator` no longer return a container, they return a navigator
3. Introducing `createAppContainer`

Closes #4722 by including flow-typed/flow-typed#2522
punk027 added a commit to punk027/jason-react-native-comp that referenced this pull request Apr 20, 2021
1. `navigationOptions` in `RouteConfig`s is now `defaultNavigationOptions`
2. `create*Navigator` no longer return a container, they return a navigator
3. Introducing `createAppContainer`

Closes #4722 by including flow-typed/flow-typed#2522
john082Coder added a commit to john082Coder/React-native-comp that referenced this pull request May 12, 2022
1. `navigationOptions` in `RouteConfig`s is now `defaultNavigationOptions`
2. `create*Navigator` no longer return a container, they return a navigator
3. Introducing `createAppContainer`

Closes #4722 by including flow-typed/flow-typed#2522
ShyJuno pushed a commit to ShyJuno/custom-react-navigation that referenced this pull request Apr 18, 2024
1. `navigationOptions` in `RouteConfig`s is now `defaultNavigationOptions`
2. `create*Navigator` no longer return a container, they return a navigator
3. Introducing `createAppContainer`

Closes #4722 by including flow-typed/flow-typed#2522
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

Successfully merging this pull request may close these issues.

None yet

5 participants