search

scala-js / scala-js-dom

Statically typed DOM API for Scala.js
Other
315 stars 160 forks source link

Fix `dom.crypto.getRandomValues` #669

Closed armanbilge closed 2 years ago

armanbilge commented 2 years ago

Will fix #668.

armanbilge commented 2 years ago

For df03142 I get the following in CI:

[error] Test org.scalajs.dom.tests.chrome.ChromeTests.cryptoGetRandomValues failed: scala.scalajs.js.JavaScriptException: TypeError: Illegal invocation, took 0.000300001 sec
[error]     at org.scalajs.dom.tests.shared.BrowserTests$class.cryptoGetRandomValues(file:///home/runner/work/scala-js-dom/scala-js-dom/tests-chrome/target/scala-2.11/testschrome-test-fastopt/main.js:3106)

[error] Test org.scalajs.dom.tests.firefox.FirefoxTests.cryptoGetRandomValues failed: scala.scalajs.js.JavaScriptException: TypeError: 'getRandomValues' called on an object that does not implement interface Crypto., took 0 sec
[error]     at org.scalajs.dom.tests.shared.BrowserTests$class.cryptoGetRandomValues(file:///home/runner/work/scala-js-dom/scala-js-dom/tests-firefox/target/scala-2.11/testsfirefox-test-fastopt/main.js:3106)
armanbilge commented 2 years ago

I assume that releasing this fix will require a bump to 2.2.0?

armanbilge commented 2 years ago

After reading https://github.com/scalacenter/sbt-version-policy/issues/71 I understand forward binary compatibility is really just an approximation used for checking backward source compatibility. So even though we are breaking forward binary compatibility here, we are not breaking source compatibility, so we can release this in 2.1.1.

sjrd commented 2 years ago

You're changing the public API (even if it's just an "added" implicit parameter) so you are potentially breaking source compatibility.

In fact, given the nature of scalajs-dom, it is extremely unlikely that there will ever be a patch release. It's virtually 100% API and no implementation, so any change triggers a minor version update.

armanbilge commented 2 years ago

Thanks, that's very helpful.