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

Fix dom.crypto.getRandomValues #669

Merged
merged 7 commits into from
Jan 19, 2022
Merged

Fix dom.crypto.getRandomValues #669

merged 7 commits into from
Jan 19, 2022

Conversation

armanbilge
Copy link
Member

Will fix #668.

@armanbilge
Copy link
Member Author

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 armanbilge requested a review from sjrd January 19, 2022 18:50
@armanbilge
Copy link
Member Author

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

@armanbilge armanbilge merged commit c6f81f4 into main Jan 19, 2022
@armanbilge armanbilge deleted the issue/668 branch January 19, 2022 19:42
@armanbilge
Copy link
Member Author

After reading scalacenter/sbt-version-policy#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
Copy link
Member

sjrd commented Jan 20, 2022

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
Copy link
Member Author

Thanks, that's very helpful.

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.

Some methods from dom.crypto are broken
3 participants