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

[Proposal] Memo in another manner #100

Closed
aralroca opened this issue Jan 9, 2019 · 6 comments
Closed

[Proposal] Memo in another manner #100

aralroca opened this issue Jan 9, 2019 · 6 comments

Comments

@aralroca
Copy link

aralroca commented Jan 9, 2019

Problem

memo approach is enough understandable. However is not 100% elegant solution... Especially if you have more "wrappers" like connect, withRouter, or other HOC:

export default memo(
  connect(mapStateToProps, mapDispatchToProps)(MyComponent),
)

Proposal

I will propose to do it following components configurations as displayName, propTypes, defaultProps...

MyComponent.displayName = 'NameOfMyComponent';

Something like:

export default function MyComponent() {
/* ... */
}


MyComponent.memo  = true;

or

export default function MyComponent() {
/* ... */
}

MyComponent.memo  = (nextProps, prevProps) => { /* ... */ };
@aralroca
Copy link
Author

aralroca commented Jan 9, 2019

Pros:

  • No more wrappers
  • Import is not necessary
  • Is posible to unify in the future together with class components as static atribute / method. Similar of shouldComponentUpdate, but static.
  • Posibility to overwrite an existing memo. (In few cases can be good to have this flexibility)

@dantman
Copy link

dantman commented Jan 9, 2019

#63 (comment)

What if component always stays the same, it would simply have a .shouldComponentUpdate prop, which—if not implemented—results in current behaviour.

This introduces an extra runtime property check for every single function component, making the whole app slower and potentially causing deoptimizations. This design was considered (see "Alternatives") and rejected.

@aralroca
Copy link
Author

aralroca commented Jan 10, 2019

I didn't imagine that this extra runtime property check can make the whole app slower. In this case, please close my proposal.

However, in the case to use memo in all components, this extra function call is getting the app slower, isn't it?

@TrySound
Copy link
Contributor

@aralroca Yes, that's why it's recommended just in a few places to fix performance holes.

@dantman
Copy link

dantman commented Jan 10, 2019

However, in the case to use memo in all components, this extra function call is getting the app slower, isn't it?

A TheComponent.memo must be checked every time a <TheComponent> may need rendering. A React.memo(TheComponent) function call is only executed once per component. And if memoization itself is done with an extra function call. That function call is only executed for components that are memoized (not all components regardless of memoization). i.e. In the situation where that extra layer is supposed to improve performance.

@aralroca
Copy link
Author

Thanks @dantman for clarification

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