From 8c2df4eb3c72dd722a4a1d1cbb48b5f806c464c1 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Mon, 2 Sep 2024 17:28:45 +0300 Subject: [PATCH] [JS] Pass args correctly to `forEach` for JS views on Kotlin maps & sets The `forEach` callback expects: - `this` to be the same as the second argument of the `forEach` call - for maps, the first arg is value, the second arg is key, the third arg is the map itself - for sets, the first arg is value, the second arg is also value, the third arg is the set itself ^KT-70707 Fixed --- .../collections__main.ts | 24 ++++++++++++----- .../js/collections/collections__main.ts | 24 ++++++++++++----- .../stdlib/js/runtime/collectionsInterop.kt | 26 +++++++++---------- 3 files changed, 49 insertions(+), 25 deletions(-) diff --git a/js/js.translator/testData/typescript-export/js/collections-in-exported-file/collections__main.ts b/js/js.translator/testData/typescript-export/js/collections-in-exported-file/collections__main.ts index dc59c343c8f09..569a4baa7e63a 100644 --- a/js/js.translator/testData/typescript-export/js/collections-in-exported-file/collections__main.ts +++ b/js/js.translator/testData/typescript-export/js/collections-in-exported-file/collections__main.ts @@ -262,14 +262,26 @@ function testMutableMap() { function joinSetOrMap(setOrMap: ReadonlySet | ReadonlyMap): string { let result = "" + const fakeThis = {}; if (setOrMap instanceof Set) { - setOrMap.forEach((a: any) => { - result += a - }); + setOrMap.forEach( + function (this: any, value: any, value2: any, set: any) { + result += value + assert(value === value2, "Problem with 'value' and 'value2' args passed to forEach callback") + assert(set === setOrMap, "Problem with passing map to forEach callback") + assert(this === fakeThis, "`this` should be passed through to forEach callback") + }, + fakeThis + ); } else { - setOrMap.forEach((key: any, value: any) => { - result += `[${key}:${value}]` - }); + setOrMap.forEach( + function (this: any, value: any, key: any, map: any) { + result += `[${key}:${value}]` + assert(map === setOrMap, "Problem with passing map to forEach callback") + assert(this === fakeThis, "`this` should be passed through to forEach callback") + }, + fakeThis, + ); } return result diff --git a/js/js.translator/testData/typescript-export/js/collections/collections__main.ts b/js/js.translator/testData/typescript-export/js/collections/collections__main.ts index dc59c343c8f09..569a4baa7e63a 100644 --- a/js/js.translator/testData/typescript-export/js/collections/collections__main.ts +++ b/js/js.translator/testData/typescript-export/js/collections/collections__main.ts @@ -262,14 +262,26 @@ function testMutableMap() { function joinSetOrMap(setOrMap: ReadonlySet | ReadonlyMap): string { let result = "" + const fakeThis = {}; if (setOrMap instanceof Set) { - setOrMap.forEach((a: any) => { - result += a - }); + setOrMap.forEach( + function (this: any, value: any, value2: any, set: any) { + result += value + assert(value === value2, "Problem with 'value' and 'value2' args passed to forEach callback") + assert(set === setOrMap, "Problem with passing map to forEach callback") + assert(this === fakeThis, "`this` should be passed through to forEach callback") + }, + fakeThis + ); } else { - setOrMap.forEach((key: any, value: any) => { - result += `[${key}:${value}]` - }); + setOrMap.forEach( + function (this: any, value: any, key: any, map: any) { + result += `[${key}:${value}]` + assert(map === setOrMap, "Problem with passing map to forEach callback") + assert(this === fakeThis, "`this` should be passed through to forEach callback") + }, + fakeThis, + ); } return result diff --git a/libraries/stdlib/js/runtime/collectionsInterop.kt b/libraries/stdlib/js/runtime/collectionsInterop.kt index adade67d0d58a..6933fbb8aa325 100644 --- a/libraries/stdlib/js/runtime/collectionsInterop.kt +++ b/libraries/stdlib/js/runtime/collectionsInterop.kt @@ -32,11 +32,11 @@ internal fun createMutableSetFrom(set: JsReadonlySet): MutableSet = @PublishedApi internal fun createMapFrom(map: JsReadonlyMap): Map = - buildMapInternal { forEach({ key, value, _ -> put(key, value) }, map) } + buildMapInternal { forEach({ value, key, _ -> put(key, value) }, map) } @PublishedApi internal fun createMutableMapFrom(map: JsReadonlyMap): MutableMap = - LinkedHashMap().apply { forEach({ key, value, _ -> put(key, value) }, map) } + LinkedHashMap().apply { forEach({ value, key, _ -> put(key, value) }, map) } internal fun createJsReadonlyArrayViewFrom(list: List): JsReadonlyArray = createJsArrayViewWith( @@ -111,7 +111,7 @@ internal fun createJsReadonlySetViewFrom(set: Set): JsReadonlySet = setContains = { v -> set.contains(v) }, valuesIterator = { createJsIteratorFrom(set.iterator()) }, entriesIterator = { createJsIteratorFrom(set.iterator()) { arrayOf(it, it) } }, - forEach = { cb, t -> forEach(cb, t) } + forEach = { callback, set, thisArg -> forEach(callback, set, thisArg) } ) internal fun createJsSetViewFrom(set: MutableSet): JsSet = @@ -123,7 +123,7 @@ internal fun createJsSetViewFrom(set: MutableSet): JsSet = setContains = { v -> set.contains(v) }, valuesIterator = { createJsIteratorFrom(set.iterator()) }, entriesIterator = { createJsIteratorFrom(set.iterator()) { arrayOf(it, it) } }, - forEach = { cb, t -> forEach(cb, t) } + forEach = { callback, set, thisArg -> forEach(callback, set, thisArg) } ) @Suppress("UNUSED_VARIABLE", "UNUSED_PARAMETER") @@ -135,7 +135,7 @@ private fun createJsSetViewWith( setContains: (E) -> Boolean, valuesIterator: () -> dynamic, entriesIterator: () -> dynamic, - forEach: (dynamic, dynamic) -> Unit, + forEach: (callback: dynamic, set: dynamic, thisArg: dynamic) -> Unit, ): dynamic { val setView = objectCreate>().also { js("it[Symbol.iterator] = valuesIterator") @@ -151,7 +151,7 @@ private fun createJsSetViewWith( keys: valuesIterator, values: valuesIterator, entries: entriesIterator, - forEach: function (cb, thisArg) { forEach(cb, thisArg || setView) } + forEach: function (cb, thisArg) { forEach(cb, setView, thisArg) } }) """) } @@ -170,7 +170,7 @@ internal fun createJsReadonlyMapViewFrom(map: Map): JsReadonlyMap forEach(cb, t) } + forEach = { callback, map, thisArg -> forEach(callback, map, thisArg) } ) internal fun createJsMapViewFrom(map: MutableMap): JsMap = @@ -184,7 +184,7 @@ internal fun createJsMapViewFrom(map: MutableMap): JsMap = keysIterator = { createJsIteratorFrom(map.keys.iterator()) }, valuesIterator = { createJsIteratorFrom(map.values.iterator()) }, entriesIterator = { createJsIteratorFrom(map.entries.iterator()) { arrayOf(it.key, it.value) } }, - forEach = { cb, t -> forEach(cb, t) } + forEach = { callback, map, thisArg -> forEach(callback, map, thisArg) } ) @Suppress("UNUSED_VARIABLE", "UNUSED_PARAMETER") @@ -198,7 +198,7 @@ private fun createJsMapViewWith( keysIterator: () -> dynamic, valuesIterator: () -> dynamic, entriesIterator: () -> dynamic, - forEach: (dynamic, dynamic) -> Unit, + forEach: (callback: dynamic, map: dynamic, thisArg: dynamic) -> Unit, ): dynamic { val mapView = objectCreate>().also { js("it[Symbol.iterator] = entriesIterator") @@ -215,7 +215,7 @@ private fun createJsMapViewWith( keys: keysIterator, values: valuesIterator, entries: entriesIterator, - forEach: function (cb, thisArg) { forEach(cb, thisArg || mapView) } + forEach: function (cb, thisArg) { forEach(cb, mapView, thisArg) } }) """) } @@ -239,13 +239,13 @@ private fun createJsIteratorFrom(iterator: Iterator, transform: (T) -> dy return jsIterator } -private fun forEach(cb: (dynamic, dynamic, dynamic) -> Unit, thisArg: dynamic) { - val iterator = thisArg.entries() +private fun forEach(cb: (dynamic, dynamic, dynamic) -> Unit, collection: dynamic, thisArg: dynamic = js("undefined")) { + val iterator = collection.entries() var result = iterator.next() while (!result.done) { val value = result.value - cb(value[0], value[1], thisArg) + cb.asDynamic().call(thisArg, value[1], value[0], collection) result = iterator.next() } } \ No newline at end of file