-
Notifications
You must be signed in to change notification settings - Fork 3k
Switch to using Set for Subscription teardowns #6400
Copy link
Copy link
Closed
Labels
8.xIssues and PRs for version 8.xIssues and PRs for version 8.x
Description
Related to this issue here: #5638
- Using an array of teardowns in our Subscriptions isn't ideal, performance-wise, for removing subscriptions.
- Using a Set, rather than an Array would more closely mimic the behavior of most other common event registries, like
EventTarget. Consider the following:
function handler() {
console.log('handled');
}
const target = new EventTarget();
target.addEventListener('unsubscribe', handler);
target.addEventListener('unsubscribe', handler);
target.addEventListener('unsubscribe', handler);
target.dispatchEvent(new Event('unsubscribe')); // Only fires `handler` once!
target.removeEventListener('unsubscribe', handler);
target.dispatchEvent(new Event('unsubscribe')); // handler not fired (as it was removed)This is in drastic contrast to our Subscription implementation, where the same subscription may be registered n times (which doesn't make sense, because it can only be torn down once), and the same function can also be registered n times, where it will execute n times.
Now, switching to using a Set internally would be a breaking change, as it would change how people can register and remove functions, more than anything. It's doubtful that it would be a breaking change for Subscriptions, because unsubscribe is idempotent.
Therefore we will not be able to do this until version 8.x.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
8.xIssues and PRs for version 8.xIssues and PRs for version 8.x