feature/mobx@next warning at /counter url

[saniko]

When opening the app at http://localhost:1337/counter
Any button click throws this warning:

Warning: forceUpdate(...): Can only update a mounted or mounting component. This usually means you called forceUpdate() on an unmounted component. This is a no-op. Please check the code for the Counter component.

FYI

[ctrlplusb]

Will be interesting to see if the latest merge of next into the mobx branch solves this.

[birkir]

Still an issue 😞

After investigating this issue better, I came to the conclusion that all packages that piggyback the componentWillMount method of an class will be frown to this issue (lots of packages man...).

Simple test:

• Add this.forceUpdate(); into your componentWillMount and see what the fuzz is all about.
• Also you can add a console.log there and you will see it fire twice on the server, twice on the client.

[ctrlplusb]

Is this only an issue when used in conjunction with react-async-component?

[diondirza]

@ctrlplusb Just running fresh pull from next branch, seems navigating to another link produce page refresh. Got an error from console related to asyncComponent.

[birkir]

@diondirza The next branch has been updated with new react-async-component so this is irrelevant error.

@ctrlplusb Yes, this only happens with react-async-component, but it does not matter if you lazy load a component or not, react-tree-walker will always fire componentWillMount twice. And this is just not to the spec of React.

[ctrlplusb]

Hey @birkir thanks for this. It definitely should not execute componentWillMount twice within react-tree-walker. That would be a bug. Have you tried latest? I updated the react-tree-walker API recently.

We should only have two componentWillMount hits. Once within react-tree-walker and then once within the render/renderToString

[ctrlplusb]

I am gonna pull down your latest mobx and investigate.

[ctrlplusb]

Busy updating the branch to latest

[ctrlplusb]

Updated to latest version of next which includes new version of react-async-component.

Works, no errors/warnings. 🎉

[birkir]

Wait, this is not really fixed, at least not the forceUpdate thing, maybe something else fixed?

A this.forceUpdate() inside componentWillMount will still trigger the error.

Also thinking... why would you want to fire componentWillMount twice. This is a lifecycle method in react components that is often used to do async work with the component.

Checkout this observer mixin for classes that hooks into componentWillMount.
https://github.com/mobxjs/mobx-react/blob/master/src/observer.js#L106-L232

edit: or is this maybe a bug in mobx-react by not checking if the component is mounted before doing any work?

[ctrlplusb]

Hmmm, you shouldn't be firing this.forceUpdate on the server though. I checked that source and they look for a __$mobxIsUnmounted which gets set via the componentWillUnmount. But on the server that lifecycle event should never get called. So perhaps this is an issue with mobx expectations vs ssr react expectations?

[ctrlplusb]

I need to play with mobx to get some more insight into this.

[ctrlplusb]

Some more details here: mobxjs/mobx-react#12

Seems to be a known issue with mobx. Even without react-async-component, calling renderToStaticMarkup/String can easily finish before an async mobx event completes, and the same would occur.

[birkir]

Oh okay, sorry to bother you with this as this seems to be non-project related issue.
Thanks for your time.