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

feat: memo separate key for reading and writing cached values #333

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hugo082
Copy link

@hugo082 hugo082 commented Dec 27, 2024

Summary

You can optionally use a separate key for reading and writing cached values by providing a setKey function.
This allows for more flexible cache management and sharing of cached values between different function calls.

For any code change,

  • Related documentation has been updated, if needed
  • Related tests have been added or updated, if needed
  • Related benchmarks have been added or updated, if needed
  • Release notes in next-minor.md or next-major.md have been added, if needed

Does this PR introduce a breaking change?

No

Bundle impact

Status File Size 1 Difference
M src/curry/memo.ts 573 +266 (+87%)

Footnotes

  1. Function size includes the import dependencies of the function.

@hugo082 hugo082 marked this pull request as ready for review December 27, 2024 10:01
@hugo082 hugo082 requested a review from aleclarson as a code owner December 27, 2024 10:01
@hugo082 hugo082 changed the title Dissociate memo cache read keys from set keys feat: memo separate key for reading and writing cached values Dec 27, 2024
@hugo082 hugo082 changed the title feat: memo separate key for reading and writing cached values feat: memo separate key for reading and writing cached values Dec 27, 2024
Copy link
Contributor

@MarlonPassos-git MarlonPassos-git left a comment

Choose a reason for hiding this comment

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

LGTM

@MarlonPassos-git MarlonPassos-git added the new feature This PR adds a new function or extends an existing one label Dec 27, 2024
@aleclarson
Copy link
Member

Hey, thanks for the PR. 🎉 Not sure about this one. Seems very niche at first glance. Especially considering the +87% size increase, we'll need to justify the feature using real world use cases (feel free to list some). Another option would be to come up with another function built purposely for this need.

@MarlonPassos-git MarlonPassos-git added the awaiting more feedback Wait on this until more people comment. label Dec 28, 2024
@hugo082
Copy link
Author

hugo082 commented Dec 31, 2024

The size increase is mostly because of the import of selectFirst, we can probably reduce with a .find(...) approach.
About the use case it's mostly to be able to share the cache accros multiple calls / parameters

@aleclarson
Copy link
Member

Hey @hugo082, good to hear from you!

Allow me to clarify what I'm asking for. A “use case” is a practical scenario where a function solves a specific problem or fulfills a need. It emphasizes why and when the function is used, rather than delving into the technical details of how it works. For instance, the use case for a capitalize() function might be standardizing user input in a form for consistent formatting.

  1. In what scenarios is it common to share the memo cache across multiple call signatures?

  2. Are these scenarios frequent enough to justify expanding the memo function, or would it be better to introduce a separate function? (This functionality may be too niche to warrant adding to Radashi at all. 🤔)

  3. Is this functionality present in another widely-used utility library, even outside of JavaScript? (While not a strict requirement, it could help demonstrate historical demand.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting more feedback Wait on this until more people comment. new feature This PR adds a new function or extends an existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants