Cross build fs2.io for Scala.js #2453

armanbilge · 2021-06-28T00:39:23Z

This PR lays the groundwork for cross-building fs2.io to Scala.js. My goal is that fs2.io on Scala.js will enable http4s ember on Scala.js in http4s/http4s#4938. I also hope this will unlock other interesting use-cases, such as command-line utilities that can take advantage of JS's fast startup times.

Specifically, I've separated out the typeclasses from the JVM-specific implementations (without compromising binary compatibility). This enables the typeclasses to be cross-built for Scala.js. A subset of the IoSuite is already passing tests on Scala.js.

At this point no JS-specific implementations are provided (besides some stubs). It remains an open question how best to simultaneously support different JS-platforms (primarily, browsers and Node.js). This work can be done piece-by-piece (net is higher priority than file at least for http4s) in follow-up PRs, or I'm also happy to continue here if so desired.

Thanks in advance for your reviews and feedback!

io/js/src/main/scala/fs2/io/net/DatagramSocketGroupPlatform.scala

armanbilge · 2021-06-28T05:53:51Z

Alas, I was being naïve thinking wishfully that Websockets === Sockets. Seeing as they're not, implementing fs2.io for browsers is a no-go, and therefore Node.js would be the canonical platform, so I think it's okay to implement specifically for Node.js. Forthcoming ...

armanbilge · 2021-06-29T17:03:32Z

Scala 3 builds are failing for now pending ScalablyTyped/Converter#202

This reverts commit baa0f52.

mpilquist · 2021-07-08T12:23:40Z

build.sbt

@@ -104,13 +104,33 @@ ThisBuild / mimaBinaryIssueFilters ++= Seq(
  ProblemFilters.exclude[ReversedMissingMethodProblem](
    "fs2.io.net.tls.TLSContext.dtlsServerBuilder"
  ),
-  ProblemFilters.exclude[Problem]("fs2.io.net.tls.TLSEngine*")
+  ProblemFilters.exclude[Problem]("fs2.io.net.tls.TLSEngine*"),
+  ProblemFilters.exclude[MissingClassProblem]("fs2.io.net.Socket$IntCallbackHandler"),


Are all of these exclusions safe?

They should be. Before we merge I will go through one more time and double-check, I can add comment justifications as well for peace-of-mind.

mpilquist · 2021-07-08T12:28:51Z

build.sbt

+    scalaJSLinkerConfig ~= (_.withModuleKind(ModuleKind.CommonJSModule)),
+    Compile / npmDependencies += "@types/node" -> "16.0.0",
+    useYarn := true,
+    stOutputPackage := "fs2.js",


Should this be fs2.io.js? Or fs2.io.jsdeps?

Whatever you want, really! Happy to make it jsdeps.

Oh, the reason I didn't put it in io ... the JS bindings could be useful outside of io?

After pondering a bit: I think the package should be fs2.internal.jsdeps and we should not expose these bindings in public APIs. There are lots of them, and because they are auto-generated by Scalably Typed which is technically still beta and they are defined by the Typescript community, I'm just scared whether we will be able to control their binary evolution. The primary purpose for adding these bindings was to help me build the internal implementations safely, not for public consumption.

For the couple places we do expose public APIs of JS-native objects, we can either:

provide our own bindings (that we control bincompat of), or

just accept js.Any (or a newtype wrapper around that) in the spirit of BYOB (bring-your-own-bindings)

@mpilquist thoughts?

If you want to use ST for this the way forward would be to use https://scalablytyped.org/docs/library-developer to generate a library which could be published independently of fs2 releases. If you use very little I'd just copy/paste the necessary parts to be completely in control.

@oyvindberg thanks for chiming in! I did find that guide very helpful when setting up the current build.

I'm still inclined to use the ST facades as internal API (it would be awesome if there's a way to automatically mark them as package-private?). I do appreciate the (continued) automation ST provides and would prefer not to chase down the copy/paste necessary to take control. Furthermore, the public APIs we provide in their place can be much more idiomatic: Chunk[Byte] instead of Buffer, callbacks A => F[B] instead of (A, B => Unit) => Unit, etc.

Aha, I just jumped in and assumed you used the default application mode - apologies.
In that case if it's fast enough to not upset anyone and marked as internal it's probably a good idea.

By package private, do you mean private[fs2] for every generated thing? I'd say that would be easy enough to add

@oyvindberg No reason to apologize for docs so good that I don't need your help 😉

Glad you agree! Yes, that's exactly what I mean by package private :)

io/js/src/main/scala/fs2/io/internal/ByteChunkOps.scala

mpilquist · 2021-07-08T12:38:17Z

io/jvm/src/main/scala/fs2/io/net/SocketPlatform.scala

-  def reads: Stream[F, Byte]
-
-  /** Indicates that this channel will not read more data. Causes `End-Of-Stream` be signalled to `available`. */
+private[net] trait SocketPlatform[F[_]] {
  def endOfInput: F[Unit]


Node doesn't support endOfInput but does support endOfOutput?

I stared at Node's Socket API for a very long time and couldn't find it.

https://nodejs.org/api/net.html#net_class_net_socket

This raises an interesting issue: for unsupported methods like this, should we

make them platform-specific

raise an exception

do nothing

(1) Causes downstreams to experience the most pain (i.e. full cross) for what could otherwise be pure crosses. But, it makes authors aware of the missing method at compile-time. (2) and (3) still allow for a pure cross, but then there may be unhappy surprises at runtime.

As a motivating example, consider this seemingly innocuous ember-client code:
https://github.com/http4s/http4s/blob/main/ember-client/src/main/scala/org/http4s/ember/client/EmberConnection.scala#L30

Actually, I think I'm all for (1). Still a bit painful, but downstreams can mitigate (1) by adding implicit syntax extensions to fill in the gaps however they like. Then, their code can purely cross-build.

armanbilge · 2021-07-09T13:43:41Z

@mpilquist thank you very much for the review! Does the approval mean you are open to merging if I address your comments? I can follow up with the missing stuff (Files API and Unix sockets) in subsequent PRs.

mpilquist · 2021-07-09T14:08:50Z

Generally okay with merging as long as you think we're in a releasable state -- I don't want to get stuck in a situation where the main branch is not releasable until some other PRs come in. Keep in mind binary compatibility too -- e.g., if we merge and then need to release a 3.0.next, then we're stuck with these APIs for a couple of years. Flip side is bincompat is easier to support on JS so maybe that's not a concern.

armanbilge · 2021-07-09T14:12:26Z

Makes sense. I think it's definitely releasable—there's no ??? gotchas, the missing features are just not there. But, happy to finish it up too. We're close!

Flip side is bincompat is easier to support on JS

I've been wondering about this ... what is bincompat for JS? ~~People don't just drop-in JS files like Jars, they do some bundling stuff right so I assume it's always recompiling?~~ UPDATE: ok, it looks like we check bincompat via the sjsir files.

I think the only new APIs I've introduced for JS are the TLSParameters. All other APIs were already public.

armanbilge · 2021-07-10T22:40:12Z

I'm marking this PR as ready-for-review, although it is missing a few things I'd say it's in a "releasable" state.

Specifically, by "releasable" I mean I feel confident about the public APIs introduced. I'd be shocked if the internal implementations don't need some fine-tuning and/or re-working so in that sense I would say this is still experimental/beta. All the test suites are happy though.

What's missing:

a file API. As discussed in Discord I will probably introduce a new NodeFiles/PosixFiles typeclass which I think warrants its own PR/discussion.
Unix sockets. This is straightforward to implement but would be helped along by a nice file API.
DTLS. Not natively supported on Node.js.

Extract JVM-specific io implementation

17f2296

diesalbla reviewed Jun 28, 2021

View reviewed changes

io/js/src/main/scala/fs2/io/net/DatagramSocketGroupPlatform.scala Outdated Show resolved Hide resolved

armanbilge mentioned this pull request Jun 28, 2021

Cross-build http4s for Scala.js http4s/http4s#4938

Merged

armanbilge added 7 commits June 29, 2021 01:45

Add Node.js types dependency

7b45d6f

Move resources to JVM directory

25f64cf

Fix build

a3e33c5

Merge branch 'main' into io-x-js

89cbcf9

Implement FileHandle for JS

4abf0b7

Fix scala 3 compile errors

6243347

Formatting

d9ad3b1

Disable fail-fast CI

baa0f52

armanbilge mentioned this pull request Jun 29, 2021

Fix typo in Stream scaladoc #2455

Merged

armanbilge added 10 commits June 29, 2021 18:47

Regenerate workflow

a71e278

Trigger CI

9106ad4

Implement Socket for Node.js

062e823

Merge branch 'main' into io-x-js

b93a8a9

Formatting

d6b5dea

Fix 2.12 compile

c5f25f6

Renaming

8779fd5

More socket implementing

e2d4256

Renaming

5640a2c

Implement UDP for Node

6194e7a

armanbilge marked this pull request as draft July 5, 2021 15:17

armanbilge added 5 commits July 6, 2021 16:16

Debugging, tcp now passing tests :)

39cbeef

Formatting

fa3b8db

Improve ScalablyTyped build

41de866

Uncomment forgotten line

0ddb2d0

UdpSuite passing on Node.js

cd57e4e

armanbilge added 10 commits July 7, 2021 03:26

Formatting

5d8463e

Bump ST plugin. Scala 3 support!

39dae92

Revert "Disable fail-fast CI"

5484614

This reverts commit baa0f52.

Regenerate workflow

49e2b52

Fix scala 3 compile

9b199cb

Refactoring

a9217c1

TLS on Node.js

0a00c18

Delete duplicated resource

25d27e9

Fix scala 3 compile

4dcea83

Update yarn.lock

78b9fd0

mpilquist approved these changes Jul 9, 2021

View reviewed changes

armanbilge added 9 commits July 9, 2021 16:20

Add JS methods to Chunk[Byte]

3a20d57

Make ST facade internal, new public APIs

9f73b5e

Formatting

a6fc080

Polish SecureContext API

f12a45d

Polish TLSParameters API

7d5540c

Fix Scala 2.12 compile

1d3671d

Fixup TLS suite

7ff5e14

Refactor UnixSockets

c62b531

Fix scala 3 compile

e1d0842

armanbilge mentioned this pull request Jul 10, 2021

Enable generated sources to be private[pkg] ScalablyTyped/Converter#329

Closed

Annotate/simplify MiMa exclusions

6614548

armanbilge marked this pull request as ready for review July 10, 2021 22:28

mpilquist merged commit 670be35 into typelevel:main Jul 11, 2021

armanbilge mentioned this pull request Jul 11, 2021

Create fs2.io.file APIs for Node.js #2478

Merged

armanbilge mentioned this pull request Jul 21, 2021

Improve Node.js Readable/Writable converters, introduce Duplex converter #2496

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cross build fs2.io for Scala.js #2453

Cross build fs2.io for Scala.js #2453

armanbilge commented Jun 28, 2021 •

edited

Loading

armanbilge commented Jun 28, 2021 •

edited

Loading

armanbilge commented Jun 29, 2021

mpilquist Jul 8, 2021

armanbilge Jul 9, 2021

armanbilge Jul 10, 2021

mpilquist Jul 8, 2021

armanbilge Jul 9, 2021

armanbilge Jul 9, 2021

armanbilge Jul 9, 2021 •

edited

Loading

oyvindberg Jul 10, 2021

armanbilge Jul 10, 2021

oyvindberg Jul 10, 2021

armanbilge Jul 10, 2021

mpilquist Jul 8, 2021

armanbilge Jul 9, 2021 •

edited

Loading

armanbilge Jul 9, 2021

armanbilge commented Jul 9, 2021

mpilquist commented Jul 9, 2021

armanbilge commented Jul 9, 2021 •

edited

Loading

armanbilge commented Jul 10, 2021

Cross build fs2.io for Scala.js #2453

Cross build fs2.io for Scala.js #2453

Conversation

armanbilge commented Jun 28, 2021 • edited Loading

armanbilge commented Jun 28, 2021 • edited Loading

armanbilge commented Jun 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armanbilge Jul 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armanbilge Jul 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

armanbilge commented Jul 9, 2021

mpilquist commented Jul 9, 2021

armanbilge commented Jul 9, 2021 • edited Loading

armanbilge commented Jul 10, 2021

armanbilge commented Jun 28, 2021 •

edited

Loading

armanbilge commented Jun 28, 2021 •

edited

Loading

armanbilge Jul 9, 2021 •

edited

Loading

armanbilge Jul 9, 2021 •

edited

Loading

armanbilge commented Jul 9, 2021 •

edited

Loading