-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
General purpose memoization tool #43538
General purpose memoization tool #43538
Conversation
@@ -0,0 +1,34 @@ | |||
type CacheMode = 'array' | 'map'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you maybe add a comment somewhere about when you should use which mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing! I have added comments, let me know if they are descriptive enough 🙏
After consideration of plans on POC v2:
I am positive we should pick Additionally, after going through its codebase, there are many short-circuit tricks in comparison algorithms where should lower level of comparison complexity. Also, I did not find any stable library for serialization which may be potential candidate. |
Great! I like it. Thanks! |
This PR is continued in #43868 |
Details
PR implements POC of memoization tool for general purpose use. The main goal is to:
Fixed Issues
$ #42200
PROPOSAL:
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop