Skip to content

Commit 3b87d8c

Browse files
authored
fix(engine-core): dedupe additions to WeakMultimap @W-21387024 (#5738)
* fix(engine-core): dedupe additions to `WeakMultimap` Also remove legacy compat code * chore: update link and fix case * chore: remove legacy note
1 parent 08a4bca commit 3b87d8c

File tree

1 file changed

+10
-42
lines changed

1 file changed

+10
-42
lines changed

packages/@lwc/engine-core/src/framework/weak-multimap.ts

Lines changed: 10 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,50 +7,16 @@
77

88
import { ArrayPush, ArraySplice, isUndefined } from '@lwc/shared';
99

10-
const supportsWeakRefs =
11-
typeof WeakRef === 'function' && typeof FinalizationRegistry === 'function';
12-
1310
/**
1411
* A map where the keys are weakly held and the values are a Set that are also each weakly held.
1512
* The goal is to avoid leaking the values, which is what would happen with a WeakMap<K, Set<V>>.
1613
*
1714
* Note that this is currently only intended to be used in dev/PRODDEBUG environments.
18-
* It leaks in legacy browsers, which may be undesired.
15+
*
16+
* This implementation relies on WeakRefs and FinalizationRegistry.
17+
* For some background, see: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakRef
1918
*/
20-
export interface WeakMultiMap<T extends object, V extends object> {
21-
get(key: T): ReadonlySet<V>;
22-
add(key: T, vm: V): void;
23-
delete(key: T): void;
24-
}
25-
26-
// In browsers that doesn't support WeakRefs, the values will still leak, but at least the keys won't
27-
class LegacyWeakMultiMap<K extends object, V extends object> implements WeakMultiMap<K, V> {
28-
private _map: WeakMap<K, Set<V>> = new WeakMap();
29-
30-
private _getValues(key: K): Set<V> {
31-
let values = this._map.get(key);
32-
if (isUndefined(values)) {
33-
values = new Set();
34-
this._map.set(key, values);
35-
}
36-
return values;
37-
}
38-
39-
get(key: K): ReadonlySet<V> {
40-
return this._getValues(key);
41-
}
42-
add(key: K, vm: V) {
43-
const set = this._getValues(key);
44-
set.add(vm);
45-
}
46-
delete(key: K) {
47-
this._map.delete(key);
48-
}
49-
}
50-
51-
// This implementation relies on the WeakRef/FinalizationRegistry proposal.
52-
// For some background, see: https://github.com/tc39/proposal-weakrefs
53-
class ModernWeakMultiMap<K extends object, V extends object> implements WeakMultiMap<K, V> {
19+
export class WeakMultiMap<K extends object, V extends object> {
5420
private _map = new WeakMap<K, WeakRef<V>[]>();
5521

5622
private _registry = new FinalizationRegistry((weakRefs: WeakRef<V>[]) => {
@@ -88,8 +54,12 @@ class ModernWeakMultiMap<K extends object, V extends object> implements WeakMult
8854
}
8955
add(key: K, value: V) {
9056
const weakRefs = this._getWeakRefs(key);
91-
// We could check for duplicate values here, but it doesn't seem worth it.
92-
// We transform the output into a Set anyway
57+
// Skip adding if already present
58+
for (const weakRef of weakRefs) {
59+
if (weakRef.deref() === value) {
60+
return;
61+
}
62+
}
9363
ArrayPush.call(weakRefs, new WeakRef<V>(value));
9464

9565
// It's important here not to leak the second argument, which is the "held value." The FinalizationRegistry
@@ -105,5 +75,3 @@ class ModernWeakMultiMap<K extends object, V extends object> implements WeakMult
10575
this._map.delete(key);
10676
}
10777
}
108-
109-
export const WeakMultiMap = supportsWeakRefs ? ModernWeakMultiMap : LegacyWeakMultiMap;

0 commit comments

Comments
 (0)