Skip to content

Commit 887ccbd

Browse files
committed
more defensive copy selectAll
1 parent 1829d8b commit 887ccbd

File tree

5 files changed

+24
-18
lines changed

5 files changed

+24
-18
lines changed

src/array.js

+2-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
export default function(x) {
2-
return typeof x === "object" && "length" in x
3-
? x // Array, TypedArray, NodeList, array-like
4-
: Array.from(x); // Map, Set, iterable, string, or anything else
1+
export default function array(x) {
2+
return x == null ? [] : Array.isArray(x) ? x : Array.from(x);
53
}

src/selectAll.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,5 +4,5 @@ import {Selection, root} from "./selection/index.js";
44
export default function(selector) {
55
return typeof selector === "string"
66
? new Selection([document.querySelectorAll(selector)], [document.documentElement])
7-
: new Selection([selector == null ? [] : array(selector)], root);
7+
: new Selection([array(selector)], root);
88
}

src/selection/data.js

+13-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {Selection} from "./index.js";
22
import {EnterNode} from "./enter.js";
3-
import array from "../array.js";
43
import constant from "../constant.js";
54

65
function bindIndex(parent, group, enter, update, exit, data) {
@@ -90,7 +89,7 @@ export default function(value, key) {
9089
var parent = parents[j],
9190
group = groups[j],
9291
groupLength = group.length,
93-
data = array(value.call(parent, parent && parent.__data__, j, parents)),
92+
data = arraylike(value.call(parent, parent && parent.__data__, j, parents)),
9493
dataLength = data.length,
9594
enterGroup = enter[j] = new Array(dataLength),
9695
updateGroup = update[j] = new Array(dataLength),
@@ -115,3 +114,15 @@ export default function(value, key) {
115114
update._exit = exit;
116115
return update;
117116
}
117+
118+
// Given some data, this returns an array-like view of it: an object that
119+
// exposes a length property and allows numeric indexing. Note that unlike
120+
// selectAll, this isn’t worried about “live” collections because the resulting
121+
// array will only be used briefly while data is being bound. (It is possible to
122+
// cause the data to change while iterating by using a key function, but please
123+
// don’t; we’d rather avoid a gratuitous copy.)
124+
function arraylike(data) {
125+
return typeof data === "object" && "length" in data
126+
? data // Array, TypedArray, NodeList, array-like
127+
: Array.from(data); // Map, Set, iterable, string, or anything else
128+
}

src/selection/selectAll.js

+1-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@ import selectorAll from "../selectorAll.js";
44

55
function arrayAll(select) {
66
return function() {
7-
var group = select.apply(this, arguments);
8-
return group == null ? [] : array(group);
7+
return array(select.apply(this, arguments));
98
};
109
}
1110

test/selection/remove-test.js

+7-9
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import assert from "assert";
2-
import {selectAll} from "../../src/index.js";
2+
import {select, selectAll} from "../../src/index.js";
33
import it from "../jsdom.js";
44

55
it("selection.remove() removes selected elements from their parent", "<h1 id='one'></h1><h1 id='two'></h1>", () => {
@@ -30,12 +30,10 @@ it("selection.remove() skips missing elements", "<h1 id='one'></h1><h1 id='two'>
3030
assert.strictEqual(two.parentNode, document.body);
3131
});
3232

33-
tape("selectChildren().remove() removes all children", function(test) {
34-
var document = jsdom("<div><span>0</span><span>1</span><span>2</span><span>3</span><span>4</span><span>5</span><span>6</span><span>7</span><span>8</span><span>9</span></div>"),
35-
p = document.querySelector("div"),
36-
selection = d3.select(p).selectChildren();
37-
test.equal(selection.size(), 10);
38-
test.equal(selection.remove(), selection);
39-
test.equal(d3.select(p).selectChildren().size(), 0);
40-
test.end();
33+
it("selectChildren().remove() removes all children", "<div><span>0</span><span>1</span><span>2</span><span>3</span><span>4</span><span>5</span><span>6</span><span>7</span><span>8</span><span>9</span></div>", () => {
34+
const p = document.querySelector("div");
35+
const selection = select(p).selectChildren();
36+
assert.strictEqual(selection.size(), 10);
37+
assert.strictEqual(selection.remove(), selection);
38+
assert.strictEqual(select(p).selectChildren().size(), 0);
4139
});

0 commit comments

Comments
 (0)