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

perf(error-boundary): optimize return of useErrorBoundaryGroup #200

Conversation

manudeli
Copy link
Member

@manudeli manudeli commented Jan 25, 2023

Overview

Optimize return of useErrorBoundaryGroup by using useMemo

AS-IS

const Comp = () => {
  // useErrorBoundaryGroup make new object, whenever Comp is rendered, for return type(group)
  const group = useErrorBoundaryGroup 

  ...
}

TO-BE

const Comp = () => {
  // useErrorBoundaryGroup don't make new object, whenever Comp is rendered, for return type(group) 
  const group = useErrorBoundaryGroup 

  ...
}

PR Checklist

  • I read and included theses actions below
  1. I have read the Contributing Guide
  2. I have written documents and tests, if needed.

@netlify
Copy link

netlify bot commented Jan 25, 2023

👷 Deploy request for slash-libraries pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit ba61e6d

@okinawaa
Copy link
Member

Thank you for your good opinion!
I have a question.

You are currently returning an object containing only reset.
If you look at the optimization you did, each time the reset changes, a new object is created. In an object that only has reset, an object is created every time the reset is changed, so is it necessary to optimize it?

Rather, I think it will cost more to optimize!

If more expensive logic is included in the object in the future, it will be effective, but at the moment, it will have a negative effect, what do you think?!

Memoization: an optimization technique used primarily to speed up computer programs by storing the results of expensive function calls and returning the cached result when the same inputs occur again.

@manudeli manudeli force-pushed the fix/error-boundary/optimize-return-useErrorBoundaryGroup branch from f840466 to 771ad25 Compare January 25, 2023 17:25
@manudeli
Copy link
Member Author

manudeli commented Jan 25, 2023

Thank you for your good opinion! I have a question.

You are currently returning an object containing only reset. If you look at the optimization you did, each time the reset changes, a new object is created. In an object that only has reset, an object is created every time the reset is changed, so is it necessary to optimize it?

Rather, I think it will cost more to optimize!

If more expensive logic is included in the object in the future, it will be effective, but at the moment, it will have a negative effect, what do you think?!

Memoization: an optimization technique used primarily to speed up computer programs by storing the results of expensive function calls and returning the cached result when the same inputs occur again.

I intended that useErrorBoundaryGroup returning object's reference need to be same until It need to be refresh during rendering.
So I add test case to prove it in 771ad25. thanks for your helping me to add test code(@ChanhyukPark-Tech)

@okinawaa
Copy link
Member

It's a good idea to keep the same reference it is semantically effective! It was nice to have a useful meeting together!

Copy link
Collaborator

@raon0211 raon0211 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@raon0211 raon0211 changed the title fix(error-boundary): optimize return of useErrorBoundaryGroup perf(error-boundary): optimize return of useErrorBoundaryGroup Jan 30, 2023
@raon0211 raon0211 merged commit afef04f into toss:main Jan 30, 2023
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

Successfully merging this pull request may close these issues.

3 participants