Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unused dependency to scalajs-dom #2681

Merged
merged 2 commits into from
Oct 23, 2021
Merged

Conversation

armanbilge
Copy link
Member

I was pretty shocked to find fs2-io the top user of scalajs-dom for browsers...

https://mvnrepository.com/artifact/org.scala-js/scalajs-dom/usages

@armanbilge
Copy link
Member Author

Darn, it's never easy.

@armanbilge armanbilge marked this pull request as draft October 17, 2021 20:32
@armanbilge
Copy link
Member Author

armanbilge commented Oct 17, 2021

So annoyingly it seems that ScalablyTyped has convoluted the stUseScalaJsDom to refer to not just scalajs-dom but also all the types in the SJS standard lib (e.g., js.Error, Uint8Array, etc.). So these all had to be switched to its own generated facades for those.

@mpilquist
Copy link
Member

Are we shading the SJS standard library now? Does the resulting artifact remain interoperable with folks using the SJS standard library?

@armanbilge
Copy link
Member Author

Are we shading the SJS standard library now?

Not really. There's just another set of facades (under our internal namespace) for the same underlying Javascript types. At the Scala level these appear to be unrelated classes, even though they are modelling the same Javascript types.

See for example this line which converts between the SJS std lib facade for Uint8Array to the ST-auto-generated facade for Uint8Array:
https://github.com/typelevel/fs2/pull/2681/files#diff-98450717b1764f56ec34f86f4c240563625aa001a68b64f062ec827c8ffe342eR36

Does the resulting artifact remain interoperable with folks using the SJS standard library?

Yes. In fact, we're still using the SJS standard library ourselves. This change affects only how we interact with our internal, private Node.js facade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants