Skip to content

Commit 4fa6e86

Browse files
authored
feat!: make insertions during iteration safe (postcss#295)
This change addresses some unexpected behavior when inserting nodes into a container. For example, assume you have a container containing two class nodes `.foo.baz`. While iterating that container with `each`, assume you are processing `.foo` at index 0. Today, the behavior is as follows: - `insertBefore(.baz, .bar)` => the next callback is to `.baz` at idx 3 - `insertAfter(.foo, .bar)` => the next callback is to `.baz` at idx 3 - `prepend(.bar)` => the next callback is to `.foo` again at idx 1 With this change, the behavior is the following, respectively: - the next callback is to `.bar` at idx 2 - the next callback is to `.bar` at idx 2 - the next callback is to `.baz` at idx 3 The newly added tests demonstrate this behavior. I've also removed the old "container#each (safe iteration)" test, as it now creates an infinite loop. I'd argue that this is the expected behavior now, though.
1 parent 1b1e9c3 commit 4fa6e86

File tree

2 files changed

+49
-14
lines changed

2 files changed

+49
-14
lines changed

src/__tests__/container.mjs

Lines changed: 44 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -35,20 +35,52 @@ test('container#each', (t) => {
3535
t.deepEqual(indexes, [0, 1]);
3636
});
3737

38-
test('container#each (safe iteration)', (t) => {
39-
let out = parse('.x, .y', (selectors) => {
40-
selectors.each((selector) => {
41-
selector.parent.insertBefore(
42-
selector,
43-
parser.className({value : 'b'})
44-
);
45-
selector.parent.insertAfter(
46-
selector,
47-
parser.className({value : 'a'})
48-
);
38+
test('container#each (safe iteration w/ insertBefore)', (t) => {
39+
let indexes = [];
40+
let out = parse('.x,.z', (selectors) => {
41+
selectors.each((selector, index) => {
42+
if (index === 0) {
43+
selectors.insertBefore(
44+
selectors.at(1),
45+
parser.className({ value: 'y' })
46+
);
47+
}
48+
indexes.push(index);
49+
});
50+
});
51+
t.deepEqual(out, '.x,.y,.z');
52+
t.deepEqual(indexes, [0, 1, 2]);
53+
});
54+
55+
test('container#each (safe iteration w/ prepend)', (t) => {
56+
let indexes = [];
57+
let out = parse('.y,.z', (selectors) => {
58+
selectors.each((selector, index) => {
59+
if (index === 0) {
60+
selectors.prepend(parser.className({ value: 'x' }));
61+
}
62+
indexes.push(index);
63+
});
64+
});
65+
t.deepEqual(out, '.x,.y,.z');
66+
t.deepEqual(indexes, [0, 2]);
67+
});
68+
69+
test('container#each (safe iteration w/ insertAfter)', (t) => {
70+
let indexes = [];
71+
let out = parse('.x,.z', (selectors) => {
72+
selectors.each((selector, index) => {
73+
if (index === 0) {
74+
selectors.insertAfter(
75+
selector,
76+
parser.className({ value: 'y' })
77+
);
78+
}
79+
indexes.push(index);
4980
});
5081
});
51-
t.deepEqual(out, '.b,.x,.a,.b, .y,.a');
82+
t.deepEqual(out, '.x,.y,.z');
83+
t.deepEqual(indexes, [0, 1, 2]);
5284
});
5385

5486
test('container#each (early exit)', (t) => {

src/selectors/container.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ export default class Container extends Node {
1818
prepend (selector) {
1919
selector.parent = this;
2020
this.nodes.unshift(selector);
21+
for ( let id in this.indexes ) {
22+
this.indexes[id]++;
23+
}
2124
return this;
2225
}
2326

@@ -82,7 +85,7 @@ export default class Container extends Node {
8285
let index;
8386
for ( let id in this.indexes ) {
8487
index = this.indexes[id];
85-
if ( oldIndex <= index ) {
88+
if ( oldIndex < index ) {
8689
this.indexes[id] = index + 1;
8790
}
8891
}
@@ -100,7 +103,7 @@ export default class Container extends Node {
100103
let index;
101104
for ( let id in this.indexes ) {
102105
index = this.indexes[id];
103-
if ( index <= oldIndex ) {
106+
if ( index >= oldIndex ) {
104107
this.indexes[id] = index + 1;
105108
}
106109
}

0 commit comments

Comments
 (0)