Add support for JSDOM JSEnv #456
Conversation
| else if ( | ||
| globalObject | ||
| .hasOwnProperty("navigator") | ||
| .asInstanceOf[Boolean] && globalObject.navigator.userAgent | ||
| .asInstanceOf[js.UndefOr[String]] | ||
| .exists(_.contains("jsdom")) | ||
| ) | ||
| JSDOMFile |
There was a problem hiding this comment.
Adapted from scala-js/scala-js-macrotask-executor#17. See the userAgent here: https://github.com/jsdom/jsdom#advanced-configuration
| private[scalajssupport] object JSDOMFile extends NodeLikeFile { | ||
| lazy val require = js.Dynamic.global.Node.constructor("return require")() | ||
| } |
There was a problem hiding this comment.
This is how we break the JSDOM sandbox. See jsdom/jsdom#2729.
There was a problem hiding this comment.
Nice catch and elegant solution! Thanks @armanbilge
|
Thanks a lot for the pr @armanbilge. Can you target the V2 branch, as all new development is going to be taking place there now since there will be no more stable releases until 2.x is released. I'll try to look at this a bit closer in the coming days as I'm extremely unfamiliar with this, and a bit iffy if we're not actually testing it at all in CI. |
|
Thanks for the response, can do!
Ok, I won't be so lazy then and I'll actually add it to CI 😅 sorry! |
There was a problem hiding this comment.
Again, thanks for the pr @armanbilge. I looked through it all and read up a bit, and I think this all seems totally fine. Just a quick note about docs and then a small question. Then we should be good to go.
|
Thanks for the fast review! :) |
There was a problem hiding this comment.
I have a couple other small things to take care of, and then I'll cut a new RC with this in. Thanks again @armanbilge!
|
Nice 🎉 Thank you! |
Relates to #183 and #212.
Specifically #212 suggests
This PR works around that, so no changes are needed upstream. /cc @viktor-podzigun