V6 Question about PureComponents

[creambyemute]

Hey there,

We just upgraded from 5.4.4 to 6.1.8 and I checked the changelogs to see if we have to adapt something.

We're still using Inject, planning on refactoring this for the new recommended way.

Question: On the Changelog for 6.0.0 there is a mention of Using PureComponent is now recommended for class-based components or React.memo for functional ones..

Does this mean, that if I have a observer Class Based Component SearchIconBase extends React.Component<Props> inject("rootStore")(observer(SearchIconBase)), that we should switch to extends React.PureComponent<Props>?

[danielkcz]

Uhh, we should probably expand that message. I know we were changing it once already but seeing it after a few months now it doesn't make any sense to me either :)

The observer will monkey patch our shouldComponentUpdate implementation if PureComponent is not used. I am not entirely sure about differences there, but yea, PureComponent is probably best.

https://github.com/mobxjs/mobx-react/blob/7d18dfdffff397dcb47f520f7f68a889dc24400e/src/observerClass.ts#L133

The observer for functional components will apply React.memo automatically.

[creambyemute]

Thx for the response, so in theory, we should be switching to PureComponent.

It's a small change, I tried it yesterday and didn't see any side-effects.

[danielkcz]

Yea, PureComponent & friends are always about optimization, it's unlikely it would break anything unless your app logic depends on optimization (bad practice imo).

I will keep this open and we should clarify the changelog further.

[danielkcz]

Related PR which improved readme regarding that, but kept the weird changelog statement.

https://github.com/mobxjs/mobx-react/pull/701

[ynejati]

I went through a similar experience upon upgrading. Ended up replacing the use of inject and creating our own provider using the context API, as recommended in the documentation. This was simple.

None of the class based components made use of shouldComponent update, except for internal, non observer, wrapper components. As long as you don't use shouldComponentUpdate, which I think was always discouraged, you should be fine.

[creambyemute]

We never used SCU, was totally fine to switch all of our 64 class components that were observers to be PureComponents now.

Inject also still seems to work for us as before, though we'll adapt to the recommended new way soon.

[danielkcz]

The inject/context has nothing to do with a problem. All related logic is within observer.

Using PureComponent is definitely better, but keep in mind it doesn't always work well for optimization purposes simply because children prop is well and will cause re-render anyway no matter if other props stay the same. Unless you memoize it in a parent of course.

[ynejati]

Sorry, those were meant to be separate. Though, technically, there was a problem with making use of the Provider component if you happen to not have the process variable in the global scope, which is what led to replacing it. But, yeah, you're right.

I guess what I'm trying to say is that in my experience, with a massive code base, upgrading from 5 to 6 was pretty seamless. I have encountered one issue that results in a stack overflow, but I have yet to prove that it was the upgrade from 5 to 6 that actually caused this.

[ynejati]

By the way, I'll update the changelog later today unless @creambyemute would like to do it.

[danielkcz]

I believe this has been resolved by #842, hopefully it's clear enough now. Closing.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs or questions.