Merge pull request #2235 from immutable-js/fix/avoid-null-when-setsize33 · immutable-js/immutable-js@009164f · GitHub
Skip to content

Commit 009164f

Browse files
jdeniaujdeniau
authored
Merge pull request #2235 from immutable-js/fix/avoid-null-when-setsize33
fix(List): preserve undefined values when grown past 32 elements
2 parents 50bf39e + 5b65bfb commit 009164f

4 files changed

Lines changed: 135 additions & 6 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 3 deletions

‎__tests__/List.ts‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,100 @@ describe('List', () => {
935935
);
936936
});
937937

938+
// A List grown past 32 elements while every value is `undefined` must read
939+
// back `undefined` at every index, never `null`.
940+
describe('preserves stored undefined (never reads back as null)', () => {
941+
// Asserts every in-bounds slot reads as `undefined` (not `null`) through
942+
// every read path: get, toArray, the values iterator (both directions),
943+
// forEach, and the structural equals / hashCode.
944+
function expectAllUndefined(list: List<number | undefined>, size: number) {
945+
const expected = List<number | undefined>(
946+
new Array(size).fill(undefined)
947+
);
948+
949+
expect(list.size).toBe(size);
950+
for (let i = 0; i < size; i++) {
951+
expect(list.get(i)).toBeUndefined();
952+
}
953+
expect(list.toArray()).toEqual(new Array(size).fill(undefined));
954+
expect([...list.values()]).toEqual(new Array(size).fill(undefined));
955+
expect([...list.toSeq().reverse().values()]).toEqual(
956+
new Array(size).fill(undefined)
957+
);
958+
list.forEach((value) => expect(value).toBeUndefined());
959+
// @ts-expect-error null should not be here
960+
expect(list.includes(null)).toBe(false);
961+
expect(list.equals(expected)).toBe(true);
962+
expect(list.hashCode()).toBe(expected.hashCode());
963+
}
964+
965+
it('setSize(32).setSize(33) — minimal repro', () => {
966+
expectAllUndefined(
967+
List<number | undefined>().setSize(32).setSize(33),
968+
33
969+
);
970+
});
971+
972+
it('setSize(1).setSize(33) — grows a tiny List across the boundary', () => {
973+
expectAllUndefined(List<number | undefined>().setSize(1).setSize(33), 33);
974+
});
975+
976+
it('setSize(31).setSize(33)', () => {
977+
expectAllUndefined(
978+
List<number | undefined>().setSize(31).setSize(33),
979+
33
980+
);
981+
});
982+
983+
it('setSize(32).setSize(65) — undefined region spanning two leaf nodes', () => {
984+
expectAllUndefined(
985+
List<number | undefined>().setSize(32).setSize(65),
986+
65
987+
);
988+
});
989+
990+
it('setSize(32).push(undefined) — grow via push, not setSize', () => {
991+
expectAllUndefined(
992+
List<number | undefined>().setSize(32).push(undefined),
993+
33
994+
);
995+
});
996+
997+
it('writing undefined past index 32 in a mutation', () => {
998+
const list = List<number | undefined>().withMutations((m) => {
999+
for (let i = 0; i < 33; i++) {
1000+
m.set(i, undefined);
1001+
}
1002+
});
1003+
expectAllUndefined(list, 33);
1004+
});
1005+
1006+
// The sequence reported in #2230, reaching the same all-undefined state
1007+
// through unshift/delete/shift rather than setSize.
1008+
it('unshift/delete/setSize/shift sequence (issue #2230)', () => {
1009+
const list = List<number | undefined>()
1010+
.unshift(86)
1011+
.unshift(87)
1012+
.unshift(54) // [54, 87, 86]
1013+
.delete(1) // [54, 86]
1014+
.setSize(6) // [54, 86, undefined, undefined, undefined, undefined]
1015+
.shift()
1016+
.shift() // [undefined, undefined, undefined, undefined]
1017+
.delete(1); // -> [undefined, undefined, undefined]
1018+
1019+
expectAllUndefined(list, 3);
1020+
});
1021+
1022+
// A genuine `null` value is a valid element and must round-trip as `null`,
1023+
// distinct from an absent / undefined slot.
1024+
it('keeps an explicitly stored null as null', () => {
1025+
const list = List<number | null>().setSize(32).setSize(33).set(0, null);
1026+
expect(list.get(0)).toBeNull();
1027+
expect(list.get(1)).toBeUndefined();
1028+
expect(list.toArray()[0]).toBeNull();
1029+
});
1030+
});
1031+
9381032
it('can be efficiently sliced', () => {
9391033
const v1 = Range(0, 2000).toList();
9401034
const v2 = v1.slice(100, -100).toList();

‎eslint.config.mjs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,12 @@ export default tseslintConfig(
144144
'jest/expect-expect': [
145145
'error',
146146
{
147-
assertFunctionNames: ['expect', 'expectIs', 'expectIsNot'],
147+
assertFunctionNames: [
148+
'expect',
149+
'expectIs',
150+
'expectIsNot',
151+
'expectAllUndefined',
152+
],
148153
additionalTestBlockFunctions: [],
149154
},
150155
],

‎src/List.js‎

Lines changed: 31 additions & 2 deletions

0 commit comments

Comments
 (0)