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

Add support for JSDOM to JSIO #529

Merged
merged 3 commits into from
Jun 7, 2022
Merged

Conversation

armanbilge
Copy link
Contributor

The JSDOM JSEnv runs in a sandbox within Node.js, so it has access to the same APIs via a sandbox-breaking technique. See:

@valencik
Copy link
Collaborator

valencik commented Jun 4, 2022

How would someone test that this works?

@armanbilge
Copy link
Contributor Author

armanbilge commented Jun 4, 2022

It's slightly annoying to setup, but we have to find a way to run the test suite in the JSDOM JSEnv in addition to Node.js.
https://github.com/scala-js/scala-js-env-jsdom-nodejs/

scoverage/scalac-scoverage-plugin#456 is one example of how to do that.

@armanbilge
Copy link
Contributor Author

There we go. Enjoy your 1000+ lines of lockfiles :P

@armanbilge armanbilge mentioned this pull request Jun 4, 2022
@armanbilge
Copy link
Contributor Author

Confirmed that JSDOM fails in CI without my changes in #530.

@valencik
Copy link
Collaborator

valencik commented Jun 5, 2022

Oh wow, thanks for that.

Is the idea that you could now run your munit tests in a browser environment?
I admit, I'm a tiny bit confused what this enables. It seems like using munit and jsdom was previously possible: https://github.com/scala-js/scala-js-env-jsdom-nodejs/#usage---writing-a-test

But this is different in that munit itself could run within the jsdom environment?

@armanbilge
Copy link
Contributor Author

Yes that's right, it was already possible to use it as a test environment. But any file based features were not working (such as diffs and line numbers) because browsers don't have access to file system. Since JSDOM is a mere sandbox emulating the browser, we can break out to access the relevant APIs.

@armanbilge
Copy link
Contributor Author

@armanbilge
Copy link
Contributor Author

Copy link
Collaborator

@valencik valencik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool. Tested locally and everything works swell!

Just noting that this only runs the tests in the JSDOM environment in our CI, there isn't a new project within sbt that runs the tests.

You can manually run them in a sbt shell by changing your jsEnv:

set testsJS / jsEnv := {new org.scalajs.jsenv.jsdomnodejs.JSDOMNodeJSEnv}`

And then run the js tests with:
testsJS / test

@valencik valencik requested review from gabro and olafurpg June 7, 2022 11:43
Copy link
Member

@olafurpg olafurpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM it's great that we can support jsdom 👏

@valencik valencik merged commit 64fee65 into scalameta:main Jun 7, 2022
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