Add Source/Sink wrappers for kotlinx-browser's ArrayBuffer#498
Add Source/Sink wrappers for kotlinx-browser's ArrayBuffer#498cedrickcooke wants to merge 2 commits intoKotlin:developfrom
Conversation
|
If these are going to be supported as sink and source targets, I think they should be in the core. The kotlinx-browser dependency isn't required, we can define our own externals to avoid having a dependency. |
| * | ||
| * @param arrayBuffer the buffer to write to. | ||
| */ | ||
| public class ArrayBufferSink( |
There was a problem hiding this comment.
I don't think either of these types should be public. If we're following the library and Kotlin conventions they should be in this shape:
fun ArrayBuffer.asSink(): RawSink
fun ArrayBuffer.asSource(): RawSourceIt's probably also worth defining this for UInt8Array directly. Or possibly even only for UInt8Array...
The corresponding Okio issue for this feature is square/okio#689
|
Done. PR/branch title are a bit out of date now, though 😆 |
- Moves implementation to kotlinx-io-core - Adds our own `external class`es instead of using kotlinx-browser types - Wrap Uint8Array instead of ArrayBuffer - Use extension functions for asSink/asSource instead of public classes
cedrickcooke
force-pushed
the
integation-kotlinx-browser
branch
from
c8a6825 to
f4fba13
Compare
| import kotlin.js.JsAny | ||
|
|
||
| /** Interop type for JavaScript `ArrayBuffer`. */ | ||
| public external class ArrayBuffer : JsAny { |
There was a problem hiding this comment.
Hmm I may have misled you by saying that this library could just copy the externals. Because normally that's what I do in libraries to avoid kotlinx-browser, but here these need to be public because the extensions hang off of them. That's... not great. I really don't like the whole kotlinx-browser approach. For Int8Array you can use Kotlin's normal ByteArray, but I'm not sure if UInt8Array is mapped to UByteArray or not. And there's no solution in the stdlib for ArrayBuffer. I will defer to the JetBrains people for what they think is the best approach.
There was a problem hiding this comment.
FWIW I do think having versions provided by the library is probably the "safest" option. Both JS/WasmJS have access to unsafeCast, which is already occasionally necessary for types that exist in both kotlinx-browser and kotlin-wrappers (like this one).
|
Related PR: #386 |
3 participants
Placed this in an
integrationfolder like Okio has, to avoid adding a dependency tocore.