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

shouldComponentUpdate does not work #1952

Closed
MikeTatsky opened this issue Apr 25, 2019 · 5 comments
Closed

shouldComponentUpdate does not work #1952

MikeTatsky opened this issue Apr 25, 2019 · 5 comments

Comments

@MikeTatsky
Copy link

MikeTatsky commented Apr 25, 2019

Hi there,
I am sorry for disturbing.
We have very complex app and we faced the problem that shouldComponentUpdate in some cases work, in some it does not with the same code.
We read that it is not recommended to use shouldComponentUpdate for MobX.
But we use ag-Grid and since it is not written on React, it can not be updated properly and fast for React over changing states. The only way to work fast for ag-Grid with React is using ag-Grid methods.
We need to rerending updating grids container.
Our code definition looks like this.

@inject('sheetStore')
@observer
export default class GridContainer extends Component {
...
Many grids rendering
...
//Accidentally it stops to work
shouldComponentUpdate = () => {
    if (sheetStore.stopReRenderingGrids) {
      return false;
    }

    return true;
}

Sorely I can not provide link on sample for this case.
Please, give me advice.

@urugator
Copy link
Collaborator

urugator commented Apr 25, 2019

Firstly don't use arrow functions () => {} for lifecycle methods.

When using @observer on a component, don't implement shouldComponentUpdate, as it will override the default implementation that MobX provides. When using mobx-react, you should in general not need to write an sCU (in our entire Mendix code base we have none). If you really need to implement sCU, split the component into two, a reactive and non-reactive (with the sCU) part, or use sections instead of observer on the entire component.

Also check mobxjs/mobx-react#667 (comment)

@ItamarShDev
Copy link
Member

https://github.com/mobxjs/mobx-react#about-shouldcomponentupdate
this is the docs segment @urugator was referencing.

@mweststrate
Copy link
Member

Was the question solved?

@MikeTatsky
Copy link
Author

@mweststrate
Thank you Michel,
We use use usual property without observable and it looks like it works so no need in shouldComponentUpdate for the moment.

@lock
Copy link

lock bot commented Jul 21, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants