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

Provide configurable Reflex object cleanup hook #609

Open
leastbad opened this issue Sep 10, 2022 · 2 comments
Open

Provide configurable Reflex object cleanup hook #609

leastbad opened this issue Sep 10, 2022 · 2 comments
Labels
enhancement New feature or request experimental javascript Pull requests that update Javascript code proposal
Milestone

Comments

@leastbad
Copy link
Contributor

Just moving this out of #592, and stashing it as a TODO issue.

As @hopsoft said in this comment:

I wonder if we should set a max limit for how many reflexes will be stored to prevent memory bloat given that the Reflex class holds references to DOM elements and Stimulus controllers. I worry that this may prevent some needed garbage collection over time. We could cleanup the references as reflexes get pushed out of the store.

Maybe we make it configurable and default to 1000? We could also leave it unlimited in debug mode?

To which I replied:

There's two high-level approaches possible:

  1. cleanup executed as part of the final stage of each Reflex
  2. launch a setInterval that runs globally, purges every n seconds (configurable via initialize option)

Both approaches would execute a callback (with a default provided), with overrides passed as an option to StimulusReflex.initialize().

Which path do you prefer?

As for the default callback, there's lots of ways to slice it:

  • do nothing
  • nuke reflex(es) right away
  • nuke reflex(es) setTimeout(n) seconds after they are finalized
  • nuke right away if not the most recent Reflex
  • nuke by FIFO queue as you suggest, with n size configurable

The cool thing is that we really could make the default () => {} and then give examples of all of the others in the docs, with an explanation about why they might want to clean up. In some ways, I like defaulting to a null function because of the principle of least surprise.

@leastbad leastbad added enhancement New feature or request proposal experimental javascript Pull requests that update Javascript code labels Sep 10, 2022
@leastbad leastbad added this to the 3.5 milestone Sep 10, 2022
@leastbad leastbad self-assigned this Sep 10, 2022
@julianrubisch
Copy link
Contributor

@marcoroth since we’re thinking about removing/reworking #592, should we consider closing this?

@marcoroth
Copy link
Member

I think we can keep #592 as-is!

We probably should just implement what's being described here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request experimental javascript Pull requests that update Javascript code proposal
Projects
None yet
Development

No branches or pull requests

3 participants