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

Make updateState available in constructor instead of componentDidMount #56

Open
GerritSe opened this issue Sep 19, 2018 · 6 comments
Open

Comments

@GerritSe
Copy link

Hey there. We recently started using react-copy-write in a small application and had a great experience so far, but there is still something that interferes with our application flow, which is the inability to call mutate before a provider is fully mounted.

https://codesandbox.io/s/oowloz2qk9

In this example the app will crash, because mutate is called before the surrounding provider is fully mounted. We want our ApiProvider to be wrapped around the whole app while still being able to use life cycle hooks to do initialization work that could mutate the state this provider gives us.

What we did for experimental reasons is to move the componentDidMount code from CopyOnWriteStoreProvider to a constructor and everything worked fine afterwards. So my question is whether it's possible to do this switch or if there are special reasons for the design decision to make updateState only available after componentDidMount of CopyOnWriteStoreProvider ran? Are there any performance implications or other pitfalls we are overlooking? If not I'd be glad to open a PR for this issue.

@kafein
Copy link

kafein commented Oct 1, 2018

@aweary Same question, is it possible to move componentDidMount code in to constructor? Don't want to use setTimeout(() => { mutate(...) }, 0)

@kafein
Copy link

kafein commented Oct 12, 2018

@aweary Sorry to rush. I need to make a decision quickly to use this library or not. Is it possible to make this happen?

@aweary
Copy link
Owner

aweary commented Oct 12, 2018

@kafein I'm not going to be doing anything else with this library until React Conf this month. If you can I'd recommend you wait until then.

@kafein
Copy link

kafein commented Oct 12, 2018

@aweary Thanks for reply. I'll wait.

@aweary
Copy link
Owner

aweary commented Nov 29, 2018

Alright, coming back to this I think moving it to constructor would be mostly fine. The issue is that the constructor may be called without the component ever being mounted (like with Suspense), so it's possible to get in a weird state where updateState is referring to a component that didn't mount.

@kafein
Copy link

kafein commented Nov 30, 2018

Hi @aweary
Thanks for explanation. I see the point, you are right. Instead of throwing error on mutate function, we can queue requests then in provider cdm lifecycle we can dequeue and call the requests. Is this reasonable?

Not sure about test but i made a pr. #61

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

No branches or pull requests

3 participants