ionspin / kotlin-multiplatform-bignum

A Kotlin multiplatform library for arbitrary precision arithmetics
Apache License 2.0
364 stars 42 forks source link

support browser-js #191

Closed BenjaminWarnke closed 2 years ago

BenjaminWarnke commented 2 years ago

I am using BigNum in my database Luposdate3000 (https://github.com/luposdate3000/luposdate3000). The database is right now targeting JVM and Browser-JS.

To be able to use BigNum (in the browser) I need this one-line patch, because otherwise it won't load. Right now, my users need to apply this patch on their own.

To actually use BigNum in an browser, it is currently necessary to add "Int64Array = BigInt64Array" to your javascript BEFORE you attempt to load BigNum. The BigNum library can than run in any browser which has the Int64Array feature. The supported Browsers can be seen at https://caniuse.com/?search=Int64Array . The underlying issue is about how the kotlin compiler is compiling "LongArray" to javascript. If you remove LongArray from the implementation, than this fix is not necessary.

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

ionspin commented 2 years ago

Hey @BenjaminWarnke , thanks for the contribution, I would love to include it in the code, but I need a bit more context.

If you remove LongArray from the implementation, than this fix is not necessary.

This might be hard, current implementation uses ULongArray as a backing array, and as far as I know in kotlin that is just an inline class represented by LongArray in runtime. There is a UIntArray based implementation, but it's not 100% up to date, and it would be a huge change to switch to it.

BenjaminWarnke commented 2 years ago

What happens if you don't apply this patch (if "commonJs" stays in place)?

According to the official documentation at https://kotlinlang.org/docs/js-modules.html we have 3 options (UMD, AMD and commonJs). commonJS is especially used in NodeJS, and does not work in the Browser. UMD (which is the default, if we remove that line) works in both environments (Browser+NodeJS) without further configuration. AMD requires further configuration to be done.

If you apply the patch, what happens if you don't include "Int64Array = BigInt64Array"?

The Browsers define the BigInt64Array class, but the Kotlin compiler expect a class called Int64Array. Without this assignment we get an error message, and can not use the library. In NodeJS we don't need this assignment.

Could we somehow include "Int64Array = BigInt64Array" in the BigNum library so that you don't have to do that manually

I had some experiments with this, but the main problem is, that we can not use any Kotlin code to do this initialization, because then it is already too late.

A possible solution could be to prepend "Int64Array = BigInt64Array" to the compiled javascript after the compiler had done its job - and before submission to maven. This of course would mess up the unit-testing, because this should use the patched javascript too.

ionspin commented 2 years ago

Great, thanks for explaining!

The Browsers define the BigInt64Array class, but the Kotlin compiler expect a class called Int64Array.

I'd suggest opening an issue on kotlin issue tracker regarding this

ionspin commented 2 years ago

Merged, once again thanks! Fix will be present in snapshots builds 0.3.3-SNAPSHOT in an hour or so.