Skip to content

Commit 2a44f34

Browse files
committed
Use WeakRef if available to allow GC to collect Range instances
1 parent cbc18d5 commit 2a44f34

File tree

10 files changed

+137
-21
lines changed

10 files changed

+137
-21
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ const xml = slimdom.serializeToWellFormedString(document);
4343

4444
Some DOM API's, such as the `DocumentFragment` constructor, require the presence of a global document, for instance to set their initial `ownerDocument` property. In these cases, slimdom will use the instance exposed through `slimdom.document`. Although you could mutate this document, it is recommended to always create your own documents (using the `Document` constructor) to avoid conflicts with other code using slimdom in your application.
4545

46-
When using a `Range`, make sure to call `detach` when you don't need it anymore. As we do not have a way to detect when a Range is no longer used, this library needs to be notified when it can stop updating the range for mutations to the surrounding nodes.
46+
When using a `Range`, make sure to call `detach` when you don't need it anymore. Unless you are only targeting environments that implement the WeakRef proposal, we do not have a way to detect when we can stop updating the range for mutations to the surrounding nodes. In environments that support WeakRef, calling detach is optional.
4747

4848
## Features and limitations
4949

api/slimdom.api.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9301,7 +9301,7 @@
93019301
{
93029302
"kind": "Method",
93039303
"canonicalReference": "slimdom!Range#detach:member(1)",
9304-
"docComment": "/**\n * Stops tracking the range.\n *\n * (non-standard) According to the spec, this method must do nothing. However, as it is not possible to rely on garbage collection to determine when to stop updating a range for node mutations, this implementation requires calling detach to stop such updates from affecting the range.\n */\n",
9304+
"docComment": "/**\n * Stops tracking the range.\n *\n * (non-standard) According to the spec, this method must do nothing. However, it is not yet possible in all browsers to allow garbage collection while keeping track of active ranges to be updated by mutations. Therefore, unless your code will only run in environments that implement the WeakRef proposal (https://github.com/tc39/proposal-weakrefs), make sure to call this method to stop updating the range and free up its resources.\n */\n",
93059305
"excerptTokens": [
93069306
{
93079307
"kind": "Content",

src/CharacterData.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ export function replaceData(
251251
(node as any)._data = newData;
252252

253253
const context = getContext(node);
254-
context._ranges.forEach((range) => {
254+
context.forEachRange((range) => {
255255
// 8. For each live range whose start node is node and start offset is greater than offset
256256
// but less than or equal to offset plus count, set its start offset to offset.
257257
if (

src/Node.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ export default abstract class Node {
188188
const currentNode = siblingsToRemove[i];
189189
const currentNodeIndex = index + i + 1;
190190

191-
context._ranges.forEach((range) => {
191+
context.forEachRange((range) => {
192192
// 6.1. For each live range whose start node is currentNode, add length to its
193193
// start offset and set its start node to node.
194194
if (range.startContainer === currentNode) {

src/Range.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import Document from './Document';
21
import Node from './Node';
32
import { getContext } from './context/Context';
43
import {
@@ -12,7 +11,6 @@ import { NodeType, isNodeOfType } from './util/NodeType';
1211
import {
1312
determineLengthOfNode,
1413
getInclusiveAncestors,
15-
getNodeDocument,
1614
getNodeIndex,
1715
getRootOfNode,
1816
} from './util/treeHelpers';
@@ -98,7 +96,7 @@ export default class Range implements AbstractRange {
9896
this.startOffset = 0;
9997
this.endContainer = context.document;
10098
this.endOffset = 0;
101-
context._ranges.push(this);
99+
context.addRange(this);
102100
}
103101

104102
/**
@@ -443,17 +441,15 @@ export default class Range implements AbstractRange {
443441
/**
444442
* Stops tracking the range.
445443
*
446-
* (non-standard) According to the spec, this method must do nothing. However, as it is not
447-
* possible to rely on garbage collection to determine when to stop updating a range for node
448-
* mutations, this implementation requires calling detach to stop such updates from affecting
449-
* the range.
444+
* (non-standard) According to the spec, this method must do nothing. However, it is not yet
445+
* possible in all browsers to allow garbage collection while keeping track of active ranges to
446+
* be updated by mutations. Therefore, unless your code will only run in environments that
447+
* implement the WeakRef proposal (https://github.com/tc39/proposal-weakrefs), make sure to call
448+
* this method to stop updating the range and free up its resources.
450449
*/
451450
detach(): void {
452451
const context = getContext(this);
453-
const index = context._ranges.indexOf(this);
454-
if (index >= 0) {
455-
context._ranges.splice(index, 1);
456-
}
452+
context.removeRange(this);
457453
}
458454

459455
/**

src/Text.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ function splitText(node: Text, offset: number): Text {
140140

141141
const indexOfNodePlusOne = getNodeIndex(node) + 1;
142142
const context = getContext(node);
143-
context._ranges.forEach((range) => {
143+
context.forEachRange((range) => {
144144
// 7.2. For each live range whose start node is node and start offset is greater than
145145
// offset, set its start node to new node and decrease its start offset by offset.
146146
if (range.startContainer === node && range.startOffset > offset) {

src/context/Context.ts

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import Text from '../Text';
1313
import XMLDocument from '../XMLDocument';
1414

1515
import NotifySet from '../mutation-observer/NotifyList';
16-
import { NodeType } from '../util/NodeType';
16+
17+
import { createWeakRef, WeakRef } from './WeakRef';
1718

1819
export type AttrConstructor = new (
1920
namespace: string | null,
@@ -49,7 +50,6 @@ export interface Context {
4950
document: Document;
5051

5152
_notifySet: NotifySet;
52-
_ranges: Range[];
5353

5454
Attr: AttrConstructor;
5555
CDATASection: CDATASectionConstructor;
@@ -63,6 +63,10 @@ export interface Context {
6363
Range: RangeConstructor;
6464
Text: TextConstructor;
6565
XMLDocument: XMLDocumentConstructor;
66+
67+
forEachRange(cb: (range: Range) => void): void;
68+
addRange(range: Range): void;
69+
removeRange(range: Range): void;
6670
}
6771

6872
/**
@@ -79,7 +83,6 @@ export class DefaultContext implements Context {
7983
* invoking their callbacks when control returns to the event loop.
8084
*/
8185
public _notifySet: NotifySet = new NotifySet();
82-
public _ranges: Range[] = [];
8386

8487
public Attr!: AttrConstructor;
8588
public CDATASection!: CDATASectionConstructor;
@@ -93,6 +96,39 @@ export class DefaultContext implements Context {
9396
public Range!: RangeConstructor;
9497
public Text!: TextConstructor;
9598
public XMLDocument!: XMLDocumentConstructor;
99+
100+
private _ranges: WeakRef<Range>[] = [];
101+
102+
public forEachRange(cb: (range: Range) => void): void {
103+
let numRanges = this._ranges.length;
104+
for (let i = numRanges - 1; i >= 0; --i) {
105+
const r = this._ranges[i].deref();
106+
if (r === undefined) {
107+
// Weak ref lost, remove
108+
this._ranges[i] = this._ranges[numRanges - 1];
109+
this._ranges.pop();
110+
numRanges -= 1;
111+
} else {
112+
cb(r);
113+
}
114+
}
115+
}
116+
117+
public addRange(range: Range): void {
118+
this._ranges.push(createWeakRef(range));
119+
}
120+
121+
public removeRange(range: Range): void {
122+
let numRanges = this._ranges.length;
123+
for (let i = numRanges - 1; i >= 0; --i) {
124+
const r = this._ranges[i].deref();
125+
if (r === undefined || r === range) {
126+
this._ranges[i] = this._ranges[numRanges - 1];
127+
this._ranges.pop();
128+
numRanges -= 1;
129+
}
130+
}
131+
}
96132
}
97133

98134
// TODO: make it possible to create multiple contexts by binding constructors to each instance

src/context/WeakRef.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// TODO: remove when interface is included in dom.d.ts typings
2+
export interface WeakRef<T> {
3+
deref(): T | undefined;
4+
}
5+
interface WeakRefConstructor<T> {
6+
new (target: T): WeakRef<T>;
7+
}
8+
declare var WeakRef: WeakRefConstructor<any>;
9+
10+
class FakeWeakRef<T> implements WeakRef<T> {
11+
private _target: T;
12+
13+
constructor(target: T) {
14+
this._target = target;
15+
}
16+
17+
public deref(): T {
18+
return this._target;
19+
}
20+
}
21+
22+
export function createWeakRef<T>(target: T): WeakRef<T> {
23+
if (typeof WeakRef === 'function') {
24+
return new WeakRef(target);
25+
}
26+
27+
return new FakeWeakRef(target);
28+
}

src/util/mutationAlgorithms.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ export function insertNode(
213213
if (child !== null) {
214214
const childIndex = getNodeIndex(child);
215215
const context = getContext(node);
216-
context._ranges.forEach((range) => {
216+
context.forEachRange((range) => {
217217
// 2.1. For each live range whose start node is parent and start offset is greater than
218218
// child’s index, increase its start offset by count.
219219
if (range.startContainer === parent && range.startOffset > childIndex) {
@@ -581,7 +581,7 @@ export function removeNode(node: Node, parent: Node, suppressObservers: boolean
581581
const index = getNodeIndex(node);
582582

583583
const context = getContext(node);
584-
context._ranges.forEach((range) => {
584+
context.forEachRange((range) => {
585585
// 2. For each live range whose start node is an inclusive descendant of node, set its start
586586
// to (parent, index).
587587
if (node.contains(range.startContainer)) {

test/Range.tests.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,4 +389,60 @@ describe('Range', () => {
389389
});
390390
});
391391
});
392+
393+
describe('WeakRef', () => {
394+
describe('fallback for older environments', () => {
395+
let oldWeakRef: any;
396+
beforeAll(() => {
397+
oldWeakRef = (global as any).WeakRef;
398+
delete (global as any).WeakRef;
399+
});
400+
afterAll(() => {
401+
(global as any).WeakRef = oldWeakRef;
402+
});
403+
404+
it('still works in older environments that do not support WeakRef', () => {
405+
range.selectNode(element);
406+
document.removeChild(element);
407+
expect(range.startOffset).toBe(0);
408+
expect(range.endOffset).toBe(0);
409+
range.detach();
410+
});
411+
});
412+
413+
describe('automatic cleanup', () => {
414+
// We can't force GC, so let's do the next best thing and stub WeakRef instead so we can
415+
// at least test the code that handles the deref returning undefined
416+
let oldWeakRef: any;
417+
let isFakeGarbageCollected: boolean;
418+
beforeAll(() => {
419+
oldWeakRef = (global as any).WeakRef;
420+
(global as any).WeakRef = class StubWeakRef<T> {
421+
constructor(private target: T) {}
422+
deref(): T | undefined {
423+
if (isFakeGarbageCollected) {
424+
return undefined;
425+
}
426+
return this.target;
427+
}
428+
};
429+
});
430+
afterAll(() => {
431+
(global as any).WeakRef = oldWeakRef;
432+
});
433+
434+
beforeEach(() => {
435+
isFakeGarbageCollected = false;
436+
});
437+
438+
it('uses WeakRef if available to automatically clean up', () => {
439+
range.selectNode(element);
440+
isFakeGarbageCollected = true;
441+
document.removeChild(element);
442+
// Range should not have been updated because our fake WeakRef said it was GC'ed
443+
expect(range.startOffset).toBe(0);
444+
expect(range.endOffset).toBe(1);
445+
});
446+
});
447+
});
392448
});

0 commit comments

Comments
 (0)