Cross-build for Scala.js #515
Conversation
| private def Hi(str: String, salt: ByteVector, iterations: Int): ByteVector = { | ||
| // TODO It is unfortunate that we have to use a sync API here when an async is available | ||
| // To make the change here will require running an F[_]: Async up the hiearchy | ||
| val salted = crypto.pbkdf2Sync(str, salt.toByteBuffer.arrayBuffer(), iterations, 8 * 32, "sha256") | ||
| bufferToByteVector(salted) | ||
| } |
There was a problem hiding this comment.
Can you elaborate? PBKDF-2 should be completely CPU bound
There was a problem hiding this comment.
Node.js offers an async version of this method with a callback. I assume they have a good reason for doing so.
My best guess would be that you are indeed right that it is CPU-bound, but it's CPU intensive enough that it would detrimentally block a single-threaded Node.js app.
|
I appreciate your effort on this but I'm not convinced that anyone actually needs it. It's a significant complication and maintenance burden going forward. Can you justify this a little bit? |
|
No worries, completely understandable! I recently got http4s cross-building to JS (including ember-server) in http4s/http4s#4938. Many people seem interested in using this with Node.js on AWS lambda and/or similar services. Skunk would be an important part of that I assume. |
|
Following up on the above, some relevant comments made in Discord. Of course, whether or not skunk falls under this umbrella is debatable :) |
This reverts commit b99cf2a.
Codecov Report
@@ Coverage Diff @@
## main #515 +/- ##
==========================================
+ Coverage 85.21% 85.22% +0.01%
==========================================
Files 121 124 +3
Lines 1589 1591 +2
Branches 47 38 -9
==========================================
+ Hits 1354 1356 +2
Misses 235 235
Continue to review full report at Codecov.
|
|
Thanks to fs2 3.1.0, all tests are passing now here. This is ready, all it needs is tpolecat/SourcePos#6. |
| // TODO It is unfortunate that we have to use a sync API here when an async is available | ||
| // To make the change here will require running an F[_]: Async up the hiearchy | ||
| val salted = crypto.pbkdf2Sync(str, byteVectorToUint8Array(salt), iterations, 8 * 32, "sha256") | ||
| bufferToByteVector(salted).take(32) |
There was a problem hiding this comment.
@mpilquist you wouldn't happen to have any guesses why this take(32) is necessary here, but not on JVM?
|
Amazing! I'm out for the day but will have a closer look later this weekend. |
|
Ok SourcePos 1.0.1 has been published. It may take a little while to appear on Central. |
|
Yep already on it :) central is fast these days! |
| import skunk._ | ||
| import natchez.Trace.Implicits.noop | ||
|
|
||
| trait SslTestPlatform { self: SslTest => |
There was a problem hiding this comment.
Why are these tests not run for JS?
There was a problem hiding this comment.
On SJS I'm not running the tests that use the Trusted SSL (which trusts all certificates). AFAICT no such configuration exists in Node.js and therefore is not in fs2.io.js either. The way you would achieve a similar configuration I think is by setting the env variable NODE_TLS_REJECT_UNAUTHORIZED=0 before running Node.
There was a problem hiding this comment.
So you just have to tell node to ignore EVERY certificate? You can't do it per connection? That's kind of nuts.
There was a problem hiding this comment.
I think you can do it on the global level with the environment variable and possibly on the socket level by fiddling with the TLS parameters: https://github.com/typelevel/fs2/blob/main/io/js/src/main/scala/fs2/io/net/tls/TLSParameters.scala#L51
But, no way to do it for an entire context.
|
Should I be concerned and/or do something about the codecov/patch failure? |
|
No, don't worry about coverage. |
|
Any last words before I slam the button? |
|
Lol nope, thanks for merging!! |
Blocked by tpolecat/SourcePos#6 (cc @ChristopherDavenport) and also
problems with scodec-bits.Update: I worked around the scodec issue. Just waiting on sourcepos and hopefully should be good to go? I'm have trouble running tests on local so going to be relying on CI for this one.This is actually very nearly a pure cross. The most notable difference for JS was resurrecting the scram implementation courtesy of @mpilquist that was subsequently scrapped in #260 as well as adding an npm dependency to saslprep.