Tram-One / tram-one

🚋 Legacy View Framework For Vanilla Javascript
http://tram-one.io/
MIT License
36 stars 8 forks source link

Clean up Unused Observable Stores (v12.0.0) #187

Closed JRJurman closed 2 years ago

JRJurman commented 2 years ago

Summary

As a side-effect of working on RedditComber a considerable performance slow down was found when interacting with the app for a period of time. This was determined to be a memory leak, caused by local stores remaining in the app even when those elements were removed from the page.

This PR fixes #185 (and additionally fixes #184)

For anyone that wants a demonstration / detailed walk-through of this PR, feel free to reach out to me on the Tram-One Discord


Changes

All the changes listed below are detailed in the comments, treat these as a high level breakdown.

Bump Major - v12.0.0

Because we now blow away state when an element is removed, this could be considered as a breaking change. While it was never intended that this state would persist in this way, it could certainly be seen as a feature. In reality, it makes more sense for people to use useGlobalStore if they want this persistent state.

If it turns out that this feature was super useful, we could consider making a new state hook, that was local to the component, but never blown away.

Main Changes

Auxiliary Changes

Testing Changes

Performance / Size Changes

See https://github.com/Tram-One/tram-one/compare/range-and-leaks...range-and-leaks-artifacts The TLDR is that the size increases by a small amount, and the performance test is just ever so slightly slower, but not significantly.

This may seem counter-intuitive given that this PR was intended to remove performance issues, but the nature of the existing performance tests don't stress this kind of behavior.

Todo

Checklist