stimulusreflex / stimulus_reflex

Build reactive applications with the Rails tooling you already know and love.
https://docs.stimulusreflex.com
MIT License
2.28k stars 172 forks source link

Provide configurable Reflex object cleanup hook #609

Open leastbad opened 2 years ago

leastbad commented 2 years ago

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:

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.

julianrubisch commented 1 year ago

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

marcoroth commented 1 year ago

I think we can keep #592 as-is!

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