Skip to content

Commit

Permalink
fix: Ensure RxJS users don't create memory leaks
Browse files Browse the repository at this point in the history
There is a bit of excitement in the RxJS community about Svelte.

- It seems like the rest of Svelte "just works™" with RxJS!
- **BUT** The danger is that unwary users will figure out how smooth this API is and accidentally create nasty memory leaks if the returned RxJS Subscriptions are not handled. Fortunately the required change is small.

NOTE: I am not entirely sure how to test this change. The goal here is to make sure that whenever you would normally teardown your store subscriptions, it is also tearing down these RxJS-shaped subscriptions. This is most commonly something you want in a component scenario. Say you have a timer component in your app that you show and remove with an `{#if}` block, when the `{#if}` block hides the component, you'd want to tear down the underlying Observable that is "ticking".

Related sveltejs#2549
  • Loading branch information
benlesh committed Apr 25, 2019
1 parent 93f7ecc commit e25b7df
Showing 1 changed file with 7 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/internal/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ export function validate_store(store, name) {
}

export function subscribe(component, store, callback) {
component.$$.on_destroy.push(store.subscribe(callback));
let unsub = store.subscribe(callback);
// Prevent memory leaks for RxJS users.
if (typeof unsub === 'object' && typeof unsub.unsubscribe === 'function') {
unsub = () => unsub.unsubscribe();
}
component.$$.on_destroy.push(unsub);
}

export function create_slot(definition, ctx, fn) {
Expand All @@ -74,4 +79,4 @@ export function exclude_internal_props(props) {
const result = {};
for (const k in props) if (k[0] !== '$') result[k] = props[k];
return result;
}
}

0 comments on commit e25b7df

Please sign in to comment.