Skip to content

feat(cache): add cleanup callback to LruCache and TtlCache#6857

Merged
kt3k merged 3 commits intodenoland:mainfrom
WasixXD:6851
Oct 29, 2025
Merged

feat(cache): add cleanup callback to LruCache and TtlCache#6857
kt3k merged 3 commits intodenoland:mainfrom
WasixXD:6851

Conversation

@WasixXD
Copy link
Copy Markdown
Contributor

@WasixXD WasixXD commented Oct 23, 2025

Closes #6851

This PR adds a option: { onEject: (ejectedKey: K, ejectedValue: V) => void) } param to LruCache and TtlCache constructor.

@WasixXD WasixXD requested a review from kt3k as a code owner October 23, 2025 16:34
@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.10%. Comparing base (4595316) to head (da8599c).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6857   +/-   ##
=======================================
  Coverage   94.09%   94.10%           
=======================================
  Files         578      578           
  Lines       42654    42676   +22     
  Branches     6781     6784    +3     
=======================================
+ Hits        40136    40159   +23     
+ Misses       2467     2466    -1     
  Partials       51       51           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lionel-rowe
Copy link
Copy Markdown
Contributor

Hmm, could work, I think it depends less on cleanness of the interface, and more on whether setting/changing/adding (/removing?) eject handlers after creation is a useful feature and/or a footgun. Also what should this do?

const cache = new LruCache(n)
    .onEject(() => console.log(1))
    .onEject(() => console.log(2))

@WasixXD
Copy link
Copy Markdown
Contributor Author

WasixXD commented Oct 25, 2025

Also what should this do?

As for now, this would just print 2

more on whether setting/changing/adding (/removing?) eject handlers after creation is a useful feature and/or a footgun

I can't think in a scenario where this would be bad

@kt3k
Copy link
Copy Markdown
Contributor

kt3k commented Oct 27, 2025

The main idea looks good to me.

I changed from a options param to a method because i think is cleaner, but I'm not attached to it.

I'd prefer this to be an option instead of a method. Making it an option would make it clearer that it only handles a single callback, not multiple ones.

@WasixXD
Copy link
Copy Markdown
Contributor Author

WasixXD commented Oct 27, 2025

I'd prefer this to be an option instead of a method. Making it an option would make it clearer that it only handles a single callback, not multiple ones.

🫡

@WasixXD WasixXD changed the title feat(cache): add onEject feat(cache): add cleanup callback to LruCache and TtlCache Oct 27, 2025
Copy link
Copy Markdown
Contributor

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

Thanks for updating! LGTM

@kt3k kt3k merged commit a0acfb0 into denoland:main Oct 29, 2025
22 checks passed
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.

LruCache and TtlCache cleanup callback when values ejected from the cache

3 participants