Skip to content
New issue

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

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

Already on GitHub? # to your account

Avoid forcing evaluation of crypto #432

Merged
merged 3 commits into from
Aug 20, 2021
Merged

Conversation

japgolly
Copy link
Contributor

Having val crypto in the package object means that when it's undefined
an exception is thrown immediately, which makes it impossible to use the
WebCrypto facade in such a case.

An example situation where this is preventative is using the WebCrypto
API from Node where its available as a library and not globally
available.

Having `val crypto` in the package object means that when it's undefined
an exception is thrown immediately, which makes it impossible to use the
WebCrypto facade in such a case.

An example situation where this is preventative is using the WebCrypto
API from Node where its available as a library and not globally
available.
@armanbilge
Copy link
Member

+1! I had this exact problem in http4s/http4s#4938.

FWIW, I think the WebCrypto facade should be its own module without any reference to DOM if possible, to maximize cross-compatibility. But not important here :)

@sjrd
Copy link
Member

sjrd commented Aug 8, 2021

For scalajs-dom 2.x, even better is to use an @js.native @JSGlobal val, like is done at
https://github.com/scala-js/scala-js-dom/pull/458/files#diff-c31f4c3c58ea375faca860d35fc697e45da565e346fc33a17bedbb9d9f92c2ff

@armanbilge armanbilge changed the base branch from master to series/1.x August 10, 2021 02:05
@japgolly japgolly marked this pull request as draft August 12, 2021 23:54
@japgolly japgolly added this to the v1.2.0 milestone Aug 13, 2021
@japgolly japgolly marked this pull request as ready for review August 20, 2021 02:19
@japgolly japgolly merged commit 968af4a into scala-js:series/1.x Aug 20, 2021
@japgolly japgolly deleted the cryptoVal branch August 20, 2021 02:23
@japgolly
Copy link
Contributor Author

Merged as this is already approved, thanks @armanbilge

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

Successfully merging this pull request may close these issues.

3 participants