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

should PureRenderMixin ignore function in props? #6601

Closed
Frezc opened this issue Apr 24, 2016 · 1 comment
Closed

should PureRenderMixin ignore function in props? #6601

Frezc opened this issue Apr 24, 2016 · 1 comment

Comments

@Frezc
Copy link

Frezc commented Apr 24, 2016

render() {
  return (
    <div>
      {
        list.map((item, index) => 
          <MyComponent
            rootRef={r => this.components[index] = r}
            otherProps={item.other}
          />
        )
      }
    </div>
  );
}

If I use PureRenderMixin in MyComponent, it will updates every time because of the rootRef. Maybe i can store the function like

if (!this.refFs[index]) this.refFs[index] = r => this.components[index] = r;
const refF = this.refFs[index];
return (
   <MyComponent
     rootRef={refF}
     otherProps={item.other}
   />
);

But it seems redundant.
If function is ignored, It will be helpful.

@jimfb
Copy link
Contributor

jimfb commented Apr 24, 2016

Yeah, we (Sebastian+Jordan+Myself) were discussing this a few months back. The problem is that the mental model of PureRenderMixin is "if props change, the component will rerender". If you ignore functions, then someone could get in the situation where they ARE changing the function (eg. capturing a variable within a closure) and the component does not update, and it becomes very hard to debug and difficult to reason about because it breaks the mental model.

That said, ignoring function changes is "usually" safe, and can improve the performance of your app. It's worth noting that PureRenderMixin is literally only a couple lines of code. If you wanted to write a version that didn't cause a rerender when function references change, it would be very easy to do so. Such a thing can be done entirely in user land. If you find it useful, I would encourage you to make it open source so other people can take advantage of the same functionality.

I'm going to close this out, which takes it off our todo list, because this is probably not something we'd do in the React core. Feel free to continue the discussion on this thread and/or build in userland.

@jimfb jimfb closed this as completed Apr 24, 2016
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

2 participants