awslabs / aws-mobile-appsync-sdk-js

JavaScript library files for Offline, Sync, Sigv4. includes support for React Native
Apache License 2.0
919 stars 266 forks source link

Rehydrated component causes an error trying to update state on an unmounted component #435

Open idanlo opened 5 years ago

idanlo commented 5 years ago

Do you want to request a feature or report a bug? report a bug

What is the current behavior? when using the Rehydrated component there is an error -

Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method.
    in Rehydrated (at router.js:28)
    in ApolloProvider (at router.js:27)
    in Unknown (at ComponentWrapper.js:29)
    in WrappedComponent (at renderApplication.js:35)
    in RCTView (at View.js:45)
    in View (at AppContainer.js:98)
    in RCTView (at View.js:45)
    in View (at AppContainer.js:115)
    in AppContainer (at renderApplication.js:34)

code -

<ApolloProvider client={client}>
  <Rehydrated>
    <App />
  </Rehydrated>
</ApolloProvider>

What is the expected behavior? Rehydrated component should wait until it rehydrates data and then render the children with no errors

Which versions and which environment (browser, react-native, nodejs) / OS are affected by this issue? Did this work in previous versions? I have only tested on react-native

edit - I'm using react-native-navigation for navigation and that code I wrote is what we use to register our screens. so each screen will be wrapped with an ApolloProvider and Rehydrated, maybe the screen switching is what causes the problem (updating state on unmounted component)

matthamil commented 5 years ago

The error occurs because the <Rehydrated /> component is awaiting its componentWillMount lifecycle and calling setState. Making that lifecycle method async isn't the source of the problem though. The problem is that because componentWillMount is async, the duration of the awaited operation could run longer than the duration of the component's lifetime. I assume the component unmounts while that componentWillMount is still in-progress. The current code in the componentWillMount should probably be moved to the componentDidMount too, since componentWillMount is going to be deprecated in a future version of React.

For the time being, you could create your own <Rehydrated /> component since its current implementation is 83 lines. Or, if I remember this comment (and come across some extra time), I could PR a fix for this. Please don't wait on me though :)

idanlo commented 5 years ago

@matthamil Thanks!

Can you tell me the difference between Rehydrated.tsx and Rehydrated-rn.tsx and how does it know which component to import when I'm doing import { Rehydrated } from 'aws-appsync-react'?

matthamil commented 5 years ago

Rehydrated.tsx is for environments that understand DOM elements. Rehydrated-rn.tsx is for React Native. Your code automagically imports the correct component due to these lines in the package.json.

idanlo commented 5 years ago

@matthamil Cool. I didn't know you can do that. Is that a react-native thing or it can be done with any package?

matthamil commented 5 years ago

@idanlo I believe this is specific to the Metro bundler (React Native's default bundler). See these lines of code in a test file in the Metro repo for some context. I'm sure other bundlers like Haul, Webpack, etc. can support something similar.

idanlo commented 5 years ago

@matthamil If the problem is only the componentWillMount I can create a pull request and change the react and react-native code to componentDidMount. Also you are importing NetInfo from react-native which has been extracted to a different package (https://github.com/react-native-community/react-native-netinfo) so I can fix that too.

does that sound good?

matthamil commented 5 years ago

I didn't write any of the code nor do I maintain it (just a wandering passerby), so I'm sure if you are able to create steps to reproduce the warning then show that your change resolves it, the maintainers will likely accept the change 👍

tugzera commented 4 years ago

Using the Rehydrate from 'aws-appsync-react' give me the waring: Possible Unhandled Promise Rejection (id: 0): TypeError: undefined is not an object (evaluating 'netinfo_1.default.isConnected.fetch')

"dependencies": { "@react-native-community/async-storage": "^1.7.1", "@react-native-community/netinfo": "^5.0.0", "amazon-cognito-identity-js": "^3.2.0", "apollo-cache-hermes": "^0.8.10", "aws-amplify": "^2.2.0", "aws-amplify-react-native": "^2.2.3", "aws-appsync": "^3.0.2", "aws-appsync-auth-link": "^2.0.1", "aws-appsync-react": "^3.0.2", "aws-appsync-subscription-link": "^2.0.1", "react": "16.9.0", "react-apollo": "^2.5.8", "react-native": "0.61.5", "reactotron-react-native": "^4.0.2" },