From 3c1ab2e2980d6cc4992558e3e869e6562cadcf2c Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 23 Dec 2022 12:26:40 +0100 Subject: [PATCH 1/2] rollback https://github.com/immutable-js/immutable-js/pull/1917 as there is an issue with circular references --- type-definitions/immutable.d.ts | 42 +++------------- type-definitions/ts-tests/deepCopy.ts | 70 --------------------------- type-definitions/ts-tests/list.ts | 3 +- type-definitions/ts-tests/map.ts | 3 +- type-definitions/ts-tests/record.ts | 9 ++-- type-definitions/ts-tests/set.ts | 3 +- 6 files changed, 20 insertions(+), 110 deletions(-) delete mode 100644 type-definitions/ts-tests/deepCopy.ts diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index 7adfdeb38..364fb78bc 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -91,30 +91,6 @@ */ declare namespace Immutable { - /** - * @ignore - */ - export type DeepCopy = T extends Collection.Keyed - ? // convert KeyedCollection to DeepCopy plain JS object - { - [key in KeyedKey extends string | number | symbol - ? KeyedKey - : string]: DeepCopy; - } - : // convert IndexedCollection or Immutable.Set to DeepCopy plain JS array - T extends Collection - ? Array> - : T extends string | number // Iterable scalar types : should be kept as is - ? T - : T extends Iterable // Iterable are converted to plain JS array - ? Array> - : T extends object // plain JS object are converted deeply - ? { - [ObjectKey in keyof T]: DeepCopy; - } - : // other case : should be kept as is - T; - /** * Lists are ordered indexed dense collections, much like a JavaScript * Array. @@ -2690,7 +2666,7 @@ declare namespace Immutable { * Note: This method may not be overridden. Objects with custom * serialization to plain JS may override toJSON() instead. */ - toJS(): DeepCopy; + toJS(): { [K in keyof TProps]: unknown }; /** * Shallowly converts this Record to equivalent native JavaScript Object. @@ -2850,7 +2826,7 @@ declare namespace Immutable { * * Converts keys to Strings. */ - toJS(): { [key in string | number | symbol]: DeepCopy }; + toJS(): { [key in string | number | symbol]: unknown }; /** * Shallowly converts this Keyed Seq to equivalent native JavaScript Object. @@ -2992,7 +2968,7 @@ declare namespace Immutable { /** * Deeply converts this Indexed Seq to equivalent native JavaScript Array. */ - toJS(): Array>; + toJS(): Array; /** * Shallowly converts this Indexed Seq to equivalent native JavaScript Array. @@ -3167,7 +3143,7 @@ declare namespace Immutable { /** * Deeply converts this Set Seq to equivalent native JavaScript Array. */ - toJS(): Array>; + toJS(): Array; /** * Shallowly converts this Set Seq to equivalent native JavaScript Array. @@ -3477,7 +3453,7 @@ declare namespace Immutable { * * Converts keys to Strings. */ - toJS(): { [key in string | number | symbol]: DeepCopy }; + toJS(): { [key in string | number | symbol]: unknown }; /** * Shallowly converts this Keyed collection to equivalent native JavaScript Object. @@ -3659,7 +3635,7 @@ declare namespace Immutable { /** * Deeply converts this Indexed collection to equivalent native JavaScript Array. */ - toJS(): Array>; + toJS(): Array; /** * Shallowly converts this Indexed collection to equivalent native JavaScript Array. @@ -3970,7 +3946,7 @@ declare namespace Immutable { /** * Deeply converts this Set collection to equivalent native JavaScript Array. */ - toJS(): Array>; + toJS(): Array; /** * Shallowly converts this Set collection to equivalent native JavaScript Array. @@ -4232,9 +4208,7 @@ declare namespace Immutable { * `Collection.Indexed`, and `Collection.Set` become `Array`, while * `Collection.Keyed` become `Object`, converting keys to Strings. */ - toJS(): - | Array> - | { [key in string | number | symbol]: DeepCopy }; + toJS(): Array | { [key in string | number | symbol]: unknown }; /** * Shallowly converts this Collection to equivalent native JavaScript Array or Object. diff --git a/type-definitions/ts-tests/deepCopy.ts b/type-definitions/ts-tests/deepCopy.ts deleted file mode 100644 index 3012e0ffc..000000000 --- a/type-definitions/ts-tests/deepCopy.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { List, Map, Record, Set, Seq, DeepCopy, Collection } from 'immutable'; - -{ - // Basic types - - // $ExpectType { a: number; b: number; } - type Test = DeepCopy<{ a: number; b: number }>; - // ^? - - // $ExpectType number - type TestA = Test['a']; - // ^? -} - -{ - // Iterables - - // $ExpectType string[] - type Test = DeepCopy; - // ^? - - // $ExpectType number[] - type Keyed = DeepCopy>; - // ^? -} - -{ - // Immutable first-level types - - // $ExpectType { [x: string]: string; } - type StringKey = DeepCopy>; - - // $ExpectType { [x: string]: object; } - type ObjectKey = DeepCopy>; - - // $ExpectType { [x: string]: object; [x: number]: object; } - type MixedKey = DeepCopy>; - - // $ExpectType string[] - type ListDeepCopy = DeepCopy>; - - // $ExpectType string[] - type SetDeepCopy = DeepCopy>; -} - -{ - // Keyed - - // $ExpectType { [x: string]: number; } - type Keyed = DeepCopy>; - - // $ExpectType { [x: string]: number; [x: number]: number; } - type KeyedMixed = DeepCopy>; - - // $ExpectType { [x: string]: number; [x: number]: number; } - type KeyedSeqMixed = DeepCopy>; - - // $ExpectType { [x: string]: number; [x: number]: number; } - type MapMixed = DeepCopy>; -} - -{ - // Nested - - // $ExpectType { map: { [x: string]: string; }; list: string[]; set: string[]; } - type NestedObject = DeepCopy<{ map: Map; list: List; set: Set; }>; - - // $ExpectType { map: { [x: string]: string; }; } - type NestedMap = DeepCopy>>; -} diff --git a/type-definitions/ts-tests/list.ts b/type-definitions/ts-tests/list.ts index e1738e9d9..1bb3a31ed 100644 --- a/type-definitions/ts-tests/list.ts +++ b/type-definitions/ts-tests/list.ts @@ -448,7 +448,8 @@ import { { // #toJS / #toJSON - // $ExpectType number[][] + // should be `number[][]` but there is an issue with deep conversion and circular references + // $ExpectType unknown[] List>().toJS(); // $ExpectType List[] diff --git a/type-definitions/ts-tests/map.ts b/type-definitions/ts-tests/map.ts index 3fffffeda..865ce237f 100644 --- a/type-definitions/ts-tests/map.ts +++ b/type-definitions/ts-tests/map.ts @@ -492,6 +492,7 @@ import { Map, List } from 'immutable'; { // #toJS - // $ExpectType { [x: string]: number; [x: number]: number; [x: symbol]: number; } + // should be `{ [x: string]: number; [x: number]: number; [x: symbol]: number; }` but there is an issue with circular references + // $ExpectType { [x: string]: unknown; [x: number]: unknown; [x: symbol]: unknown; } Map().toJS(); } diff --git a/type-definitions/ts-tests/record.ts b/type-definitions/ts-tests/record.ts index 7a626e438..3b501dfd4 100644 --- a/type-definitions/ts-tests/record.ts +++ b/type-definitions/ts-tests/record.ts @@ -27,7 +27,8 @@ import { List, Map, Record, Set } from 'immutable'; // $ExpectError pointXY.y = 10; - // $ExpectType { x: number; y: number; } + // should be `{ x: number; y: number; }` but there is an issue with circular references + // $ExpectType { x: unknown; y: unknown; } pointXY.toJS(); class PointClass extends PointXY { @@ -60,7 +61,8 @@ import { List, Map, Record, Set } from 'immutable'; // $ExpectType { x: number; y: number; } point.toJSON(); - // $ExpectType { x: number; y: number; } + // should be `{ x: number; y: number; }` but there is an issue with circular references + // $ExpectType { x: unknown; y: unknown; } point.toJS(); } @@ -88,6 +90,7 @@ import { List, Map, Record, Set } from 'immutable'; // $ExpectType { map: Map; list: List; set: Set; } withMap.toJSON(); - // $ExpectType { map: { [x: string]: string; }; list: string[]; set: string[]; } + // should be `{ map: { [x: string]: string; }; list: string[]; set: string[]; }` but there is an issue with circular references + // $ExpectType { map: unknown; list: unknown; set: unknown; } withMap.toJS(); } diff --git a/type-definitions/ts-tests/set.ts b/type-definitions/ts-tests/set.ts index 596fb0b1b..52af68de2 100644 --- a/type-definitions/ts-tests/set.ts +++ b/type-definitions/ts-tests/set.ts @@ -286,7 +286,8 @@ import { Set, Map } from 'immutable'; { // #toJS / #toJJSON - // $ExpectType number[][] + // should be `number[][]` but there is an issue with circular references + // $ExpectType unknown[] Set>().toJS(); // $ExpectType Set[] From a65b22fa27f00899688dbcb3c2c2df747aebc8a1 Mon Sep 17 00:00:00 2001 From: Julien Deniau Date: Fri, 23 Dec 2022 14:23:17 +0100 Subject: [PATCH 2/2] rollback some of the change on `toJS` to avoid circular reference --- CHANGELOG.md | 4 ++ type-definitions/immutable.d.ts | 52 +++++++++++++++--- type-definitions/ts-tests/deepCopy.ts | 79 +++++++++++++++++++++++++++ type-definitions/ts-tests/list.ts | 3 +- type-definitions/ts-tests/map.ts | 3 +- type-definitions/ts-tests/record.ts | 6 +- type-definitions/ts-tests/set.ts | 3 +- 7 files changed, 132 insertions(+), 18 deletions(-) create mode 100644 type-definitions/ts-tests/deepCopy.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 1369d0638..72f13794c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). Dates are formatted as YYYY-MM-DD. +## Unreleased + +- [Typescript] rollback some of the change on `toJS` to avoir circular reference + ## [4.2.0] - 2022-12-22 - [TypeScript] Better type for toJS [#1917](https://github.com/immutable-js/immutable-js/pull/1917) by [jdeniau](https://github.com/jdeniau) diff --git a/type-definitions/immutable.d.ts b/type-definitions/immutable.d.ts index 364fb78bc..bf21c42bf 100644 --- a/type-definitions/immutable.d.ts +++ b/type-definitions/immutable.d.ts @@ -91,6 +91,40 @@ */ declare namespace Immutable { + /** + * @ignore + * + * Used to convert deeply all immutable types to a plain TS type. + * Using `unknown` on object instead of recursive call as we have a circular reference issue + */ + export type DeepCopy = T extends Record + ? // convert Record to DeepCopy plain JS object + { + [key in keyof R]: R[key] extends object ? unknown : R[key]; + } + : T extends Collection.Keyed + ? // convert KeyedCollection to DeepCopy plain JS object + { + [key in KeyedKey extends string | number | symbol + ? KeyedKey + : string]: V extends object ? unknown : V; + } + : // convert IndexedCollection or Immutable.Set to DeepCopy plain JS array + T extends Collection + ? Array + : T extends string | number // Iterable scalar types : should be kept as is + ? T + : T extends Iterable // Iterable are converted to plain JS array + ? Array + : T extends object // plain JS object are converted deeply + ? { + [ObjectKey in keyof T]: T[ObjectKey] extends object + ? unknown + : T[ObjectKey]; + } + : // other case : should be kept as is + T; + /** * Lists are ordered indexed dense collections, much like a JavaScript * Array. @@ -2666,7 +2700,7 @@ declare namespace Immutable { * Note: This method may not be overridden. Objects with custom * serialization to plain JS may override toJSON() instead. */ - toJS(): { [K in keyof TProps]: unknown }; + toJS(): DeepCopy; /** * Shallowly converts this Record to equivalent native JavaScript Object. @@ -2826,7 +2860,7 @@ declare namespace Immutable { * * Converts keys to Strings. */ - toJS(): { [key in string | number | symbol]: unknown }; + toJS(): { [key in string | number | symbol]: DeepCopy }; /** * Shallowly converts this Keyed Seq to equivalent native JavaScript Object. @@ -2968,7 +3002,7 @@ declare namespace Immutable { /** * Deeply converts this Indexed Seq to equivalent native JavaScript Array. */ - toJS(): Array; + toJS(): Array>; /** * Shallowly converts this Indexed Seq to equivalent native JavaScript Array. @@ -3143,7 +3177,7 @@ declare namespace Immutable { /** * Deeply converts this Set Seq to equivalent native JavaScript Array. */ - toJS(): Array; + toJS(): Array>; /** * Shallowly converts this Set Seq to equivalent native JavaScript Array. @@ -3453,7 +3487,7 @@ declare namespace Immutable { * * Converts keys to Strings. */ - toJS(): { [key in string | number | symbol]: unknown }; + toJS(): { [key in string | number | symbol]: DeepCopy }; /** * Shallowly converts this Keyed collection to equivalent native JavaScript Object. @@ -3635,7 +3669,7 @@ declare namespace Immutable { /** * Deeply converts this Indexed collection to equivalent native JavaScript Array. */ - toJS(): Array; + toJS(): Array>; /** * Shallowly converts this Indexed collection to equivalent native JavaScript Array. @@ -3946,7 +3980,7 @@ declare namespace Immutable { /** * Deeply converts this Set collection to equivalent native JavaScript Array. */ - toJS(): Array; + toJS(): Array>; /** * Shallowly converts this Set collection to equivalent native JavaScript Array. @@ -4208,7 +4242,9 @@ declare namespace Immutable { * `Collection.Indexed`, and `Collection.Set` become `Array`, while * `Collection.Keyed` become `Object`, converting keys to Strings. */ - toJS(): Array | { [key in string | number | symbol]: unknown }; + toJS(): + | Array> + | { [key in string | number | symbol]: DeepCopy }; /** * Shallowly converts this Collection to equivalent native JavaScript Array or Object. diff --git a/type-definitions/ts-tests/deepCopy.ts b/type-definitions/ts-tests/deepCopy.ts new file mode 100644 index 000000000..2f16c983d --- /dev/null +++ b/type-definitions/ts-tests/deepCopy.ts @@ -0,0 +1,79 @@ +import { List, Map, Record, Set, Seq, DeepCopy, Collection } from 'immutable'; + +{ + // Basic types + + // $ExpectType { a: number; b: number; } + type Test = DeepCopy<{ a: number; b: number }>; +} + +{ + // Iterables + + // $ExpectType string[] + type Test = DeepCopy; + + // $ExpectType number[] + type Keyed = DeepCopy>; +} + +{ + // Immutable first-level types + + // $ExpectType { [x: string]: string; } + type StringKey = DeepCopy>; + + // should be `{ [x: string]: object; }` but there is an issue with circular references + // $ExpectType { [x: string]: unknown; } + type ObjectKey = DeepCopy>; + + // should be `{ [x: string]: object; [x: number]: object; }` but there is an issue with circular references + // $ExpectType { [x: string]: unknown; [x: number]: unknown; } + type MixedKey = DeepCopy>; + + // $ExpectType string[] + type ListDeepCopy = DeepCopy>; + + // $ExpectType string[] + type SetDeepCopy = DeepCopy>; +} + +{ + // Keyed + + // $ExpectType { [x: string]: number; } + type Keyed = DeepCopy>; + + // $ExpectType { [x: string]: number; [x: number]: number; } + type KeyedMixed = DeepCopy>; + + // $ExpectType { [x: string]: number; [x: number]: number; } + type KeyedSeqMixed = DeepCopy>; + + // $ExpectType { [x: string]: number; [x: number]: number; } + type MapMixed = DeepCopy>; +} + +{ + // Nested + + // should be `{ map: { [x: string]: string; }; list: string[]; set: string[]; }` but there is an issue with circular references + // $ExpectType { map: unknown; list: unknown; set: unknown; } + type NestedObject = DeepCopy<{ map: Map; list: List; set: Set; }>; + + // should be `{ map: { [x: string]: string; }; }`, but there is an issue with circular references + // $ExpectType { map: unknown; } + type NestedMap = DeepCopy>>; +} + +{ + // Circular references + + type Article = Record<{ title: string; tag: Tag; }>; + type Tag = Record<{ name: string; article: Article; }>; + + // should handle circular references here somehow + // $ExpectType { title: string; tag: unknown; } + type Circular = DeepCopy
; + // ^? +} diff --git a/type-definitions/ts-tests/list.ts b/type-definitions/ts-tests/list.ts index 1bb3a31ed..e1738e9d9 100644 --- a/type-definitions/ts-tests/list.ts +++ b/type-definitions/ts-tests/list.ts @@ -448,8 +448,7 @@ import { { // #toJS / #toJSON - // should be `number[][]` but there is an issue with deep conversion and circular references - // $ExpectType unknown[] + // $ExpectType number[][] List>().toJS(); // $ExpectType List[] diff --git a/type-definitions/ts-tests/map.ts b/type-definitions/ts-tests/map.ts index 865ce237f..3fffffeda 100644 --- a/type-definitions/ts-tests/map.ts +++ b/type-definitions/ts-tests/map.ts @@ -492,7 +492,6 @@ import { Map, List } from 'immutable'; { // #toJS - // should be `{ [x: string]: number; [x: number]: number; [x: symbol]: number; }` but there is an issue with circular references - // $ExpectType { [x: string]: unknown; [x: number]: unknown; [x: symbol]: unknown; } + // $ExpectType { [x: string]: number; [x: number]: number; [x: symbol]: number; } Map().toJS(); } diff --git a/type-definitions/ts-tests/record.ts b/type-definitions/ts-tests/record.ts index 3b501dfd4..38da1148e 100644 --- a/type-definitions/ts-tests/record.ts +++ b/type-definitions/ts-tests/record.ts @@ -27,8 +27,7 @@ import { List, Map, Record, Set } from 'immutable'; // $ExpectError pointXY.y = 10; - // should be `{ x: number; y: number; }` but there is an issue with circular references - // $ExpectType { x: unknown; y: unknown; } + // $ExpectType { x: number; y: number; } pointXY.toJS(); class PointClass extends PointXY { @@ -61,8 +60,7 @@ import { List, Map, Record, Set } from 'immutable'; // $ExpectType { x: number; y: number; } point.toJSON(); - // should be `{ x: number; y: number; }` but there is an issue with circular references - // $ExpectType { x: unknown; y: unknown; } + // $ExpectType { x: number; y: number; } point.toJS(); } diff --git a/type-definitions/ts-tests/set.ts b/type-definitions/ts-tests/set.ts index 52af68de2..596fb0b1b 100644 --- a/type-definitions/ts-tests/set.ts +++ b/type-definitions/ts-tests/set.ts @@ -286,8 +286,7 @@ import { Set, Map } from 'immutable'; { // #toJS / #toJJSON - // should be `number[][]` but there is an issue with circular references - // $ExpectType unknown[] + // $ExpectType number[][] Set>().toJS(); // $ExpectType Set[]