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

refact: remove memoize #4

Conversation

brandon-ja
Copy link
Contributor

@brandon-ja brandon-ja commented Dec 26, 2023

So it turns out memorize has a bunch of problems that are not easy to fix.

  • Currently the internal cache is leaky meaning entries never exit the cache once they get added.
  • Adding a cleanup method isn't super easy given you need to configure how often the cleanup logic runs.
  • Even if a cleanup method is added it will likely be in a go-routine which we need a context to handle otherwise we risk leaking that go routine.
  • Supporting deletes for persist.Store types would require adding more complexity to that interface, further complicating every implementation of the persist.Store
  • This package uses functional style code, which is fine when simple, but hand to integrate into non-functional code as it's complexity grows.

Rather than try to solve these issues, I think removing this package is the better option. This will allow the cachin to stay simple, functional and extensible.

Copy link

linear bot commented Dec 26, 2023

@brandon-ja brandon-ja merged commit e14b9e3 into main Jan 3, 2024
2 of 3 checks passed
@brandon-ja brandon-ja deleted the brandonatkinson-devx-5678-add-the-ability-to-reset-the-ttl-whenever-a-file-is-read-in-2 branch January 3, 2024 18:55
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