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

Add weakMapMemoize utility. #26605

Merged
merged 15 commits into from
Dec 20, 2024
Merged

Add weakMapMemoize utility. #26605

merged 15 commits into from
Dec 20, 2024

Conversation

salazarm
Copy link
Contributor

Summary & Motivation

Introduces a memoize function thats more powerful than what lodash offers. Lodash memoize only works with string keys, and so it requires you to take your arguments and somehow serialize it to a string. Depending on the object this can be very slow if the object is big. To avoid the headache of serialization I'm adding a weakMapMemoize which is much more flexible and can cache any function mixing both primitives and objects.

Going to use this to memoize the filterByQuery helpers for the GanttChart since it looks like we call the utility in a lot of different places separately and having them coordinate so that there's only 1 call is a bit tricky.

How I Tested These Changes

jest tests

Copy link

github-actions bot commented Dec 19, 2024

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-hain2gzjt-elementl.vercel.app
https://salazarm-weakMapMemoize.core-storybook.dagster-docs.io

Built with commit 3715f68.
This pull request is being automatically deployed with vercel-action

@salazarm salazarm requested a review from prha December 20, 2024 15:48
Copy link
Member

@hellendag hellendag left a comment

Choose a reason for hiding this comment

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

Cool, seems reasonable to me.

@salazarm salazarm merged commit d5d70ca into master Dec 20, 2024
1 of 2 checks passed
@salazarm salazarm deleted the salazarm/weakMapMemoize branch December 20, 2024 22:21
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.

2 participants