From 2ec55e653ab4d8e6e78af331391436d6ee4972d9 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Mon, 22 Jul 2024 08:32:08 +0000 Subject: [PATCH 1/4] Try to fix issue with empty list that is a singleton --- __tests__/List.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/__tests__/List.ts b/__tests__/List.ts index 2cb28ee6a6..ac2d89e3ef 100644 --- a/__tests__/List.ts +++ b/__tests__/List.ts @@ -927,6 +927,30 @@ describe('List', () => { expect(isNaNValue(list.get(0))).toBe(true); }); + it('return a new emptyList if the emptyList has been mutated #2003', () => { + const emptyList = List(); + + const nonEmptyList = emptyList.withMutations(l => { + l.setSize(1); + l.set(0, 'a'); + }); + + expect(nonEmptyList.size).toBe(1); + expect(nonEmptyList).toEqual(List.of('a')); + expect(emptyList.size).toBe(0); + + const mutableList = emptyList.asMutable(); + + mutableList.setSize(1); + mutableList.set(0, 'b'); + + expect(mutableList.size).toBe(1); + expect(mutableList).toEqual(List.of('b')); + + expect(emptyList.size).toBe(0); + expect(List().size).toBe(0); + }); + // Note: NaN is the only value not equal to itself. The isNaN() built-in // function returns true for any non-numeric value, not just the NaN value. function isNaNValue(value) { From d41093253403612ff05bb492681a8bf2e5c4896b Mon Sep 17 00:00:00 2001 From: Julien Deniau <1398469+jdeniau@users.noreply.github.com> Date: Mon, 22 Jul 2024 15:15:50 +0200 Subject: [PATCH 2/4] Update List.ts --- __tests__/List.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/__tests__/List.ts b/__tests__/List.ts index ac2d89e3ef..c050d859cc 100644 --- a/__tests__/List.ts +++ b/__tests__/List.ts @@ -951,6 +951,13 @@ describe('List', () => { expect(List().size).toBe(0); }); + it('Mutating empty list with a JS API should not mutate new instances', () => { + Object.assign(List(), List([1, 2])); + + expect(List().size).toBe(0); + expect(List().toArray()).toEqual([]); + }); + // Note: NaN is the only value not equal to itself. The isNaN() built-in // function returns true for any non-numeric value, not just the NaN value. function isNaNValue(value) { From 899777144302e064673daef09c6bc24ccead05a4 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Tue, 24 Sep 2024 21:56:29 +0000 Subject: [PATCH 3/4] Freeze emptyList to avoid mutation --- __tests__/List.ts | 6 +++++- src/List.js | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/__tests__/List.ts b/__tests__/List.ts index c050d859cc..c4b601157d 100644 --- a/__tests__/List.ts +++ b/__tests__/List.ts @@ -952,7 +952,11 @@ describe('List', () => { }); it('Mutating empty list with a JS API should not mutate new instances', () => { - Object.assign(List(), List([1, 2])); + expect(() => { + Object.assign(List(), List([1, 2])); + }).toThrow( + "Cannot assign to read only property 'size' of object '[object Object]'" + ); expect(List().size).toBe(0); expect(List().toArray()).toEqual([]); diff --git a/src/List.js b/src/List.js index e512bb90db..f88aa61637 100644 --- a/src/List.js +++ b/src/List.js @@ -410,7 +410,11 @@ function makeList(origin, capacity, level, root, tail, ownerID, hash) { let EMPTY_LIST; export function emptyList() { - return EMPTY_LIST || (EMPTY_LIST = makeList(0, 0, SHIFT)); + if (!EMPTY_LIST) { + EMPTY_LIST = makeList(0, 0, SHIFT); + Object.freeze(EMPTY_LIST); + } + return EMPTY_LIST; } function updateList(list, index, value) { From 14631a62274c5c8f1ecf7e3de491d0fe924e7ecc Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Tue, 24 Sep 2024 22:12:52 +0000 Subject: [PATCH 4/4] Do not keep a singletin of EMPTY_LIST --- __tests__/List.ts | 8 ++------ src/List.js | 7 +------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/__tests__/List.ts b/__tests__/List.ts index c4b601157d..1fa808b807 100644 --- a/__tests__/List.ts +++ b/__tests__/List.ts @@ -829,7 +829,7 @@ describe('List', () => { it('chained mutations does not result in new empty list instance', () => { const v1 = List(['x']); const v2 = v1.withMutations(v => v.push('y').pop().pop()); - expect(v2).toBe(List()); + expect(v2).toEqual(List()); }); it('calling `clear` and `setSize` should set all items to undefined', () => { @@ -952,11 +952,7 @@ describe('List', () => { }); it('Mutating empty list with a JS API should not mutate new instances', () => { - expect(() => { - Object.assign(List(), List([1, 2])); - }).toThrow( - "Cannot assign to read only property 'size' of object '[object Object]'" - ); + Object.assign(List(), List([1, 2])); expect(List().size).toBe(0); expect(List().toArray()).toEqual([]); diff --git a/src/List.js b/src/List.js index f88aa61637..be698316d3 100644 --- a/src/List.js +++ b/src/List.js @@ -408,13 +408,8 @@ function makeList(origin, capacity, level, root, tail, ownerID, hash) { return list; } -let EMPTY_LIST; export function emptyList() { - if (!EMPTY_LIST) { - EMPTY_LIST = makeList(0, 0, SHIFT); - Object.freeze(EMPTY_LIST); - } - return EMPTY_LIST; + return makeList(0, 0, SHIFT); } function updateList(list, index, value) {