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

Provide Node.js interface through global object in JSDOMNodeJSEnv #197

Closed
viktor-podzigun opened this issue Jan 9, 2018 · 10 comments
Closed

Comments

@viktor-podzigun
Copy link

Currently, in JSDOMNodeJSEnv (which is the case when requiresDOM in Test := true) there is no Node.js global object exposed.

Though, in plain NodeJSEnv it is available and can be used as a way to detect/use Node.js environment described here.

As a result there is no way for test frameworks/plugins (like scoverage, for example) to access/use Node internal/system modules (require("fs") and require("path"), for example).

Motivation:
Fixing this issue would be a step forward to make scoverage work with scalajs-bundler together with corresponding fix for this issue in the scoverage-plugin

@sjrd
Copy link
Collaborator

sjrd commented Jan 9, 2018

JSDOMNodeJSEnv is meant to provide a browser-like environment, and not expose Node.js-specific stuff.

Maybe what you need is a dedicated, custom JSEnv that provides an escape hatch for scoverage to access a portion of the filesystem. Or else use a local XMLHttpRequest-based connection to an sbt-plugin-provided server to handle the filesystem operations.

@viktor-podzigun
Copy link
Author

From a point of view of scoverage plugin (or any other sbt-plugin that needs node system modules access) the interface should be the same.

Since NodeJSEnv already works out of the box through global object, what if we create a third, custom JSDOMNodeJSSystemEnv that will expose global object, and keeping JSDOMNodeJSEnv unmodified for browser-like environment?

@viktor-podzigun
Copy link
Author

Or maybe better adding a configuration parameter for existing JSDOMNodeJSEnv to expose global or not.

@sjrd
Copy link
Collaborator

sjrd commented Jan 9, 2018

You can define your own JSDOMNodeJSSystemEnv as a separate library, which is what I would recommend for now. All our JSEnvs are now distributed as separate libraries anyway, so doing so does not make yours any less first-class than the others.

@nimaeskandary
Copy link

Has this been fixed / is there a workaround to make scoverage work?

@viktor-podzigun
Copy link
Author

Hi @sjrd,
since we have a custom copy of JSDOMNodeJSEnv inside scalajs-bundler, would it work to solve the current issue by adding sbt setting, something like exposeNodejsGlobal in Test := true?
What do you think?

@sjrd
Copy link
Collaborator

sjrd commented Mar 26, 2019

If you ask me, I believe sbt settings are already proliferating too fast in this plugin. I wouldn't recommend adding any other setting unless absolutely necessary. In this instance, at the very least it can be a configuration option of the JSEnv subclass, like withSourceMap for the official NodeJSEnv.

@viktor-podzigun
Copy link
Author

Thank you @sjrd for your quick feedback.

Will try to prepare PR for org.scalajs.jsenv.jsdomnodejs.JSDOMNodeJSEnv then.

And how can we apply it for scalajs-bundler?
Since its instantiated inside the plugin ScalaJSBundlerPlugin.scala#L790 and defined by the requireJsDomEnv boolean setting.

Maybe we can merge these settings into single one, like:

nodejsEnvConfig := NodejsEnvConfig(
  requireJsDom = true,
  exposeGlobal = true,
  ...
)

Does it make sense?

BTW, scala.js apps build for Electron would also benefit from having the way to expose nodejs global inside jsdom env, since Electron exposes full access to Node.js both in the main and the renderer process.

@sjrd
Copy link
Collaborator

sjrd commented Mar 26, 2019

Since its instantiated inside the plugin ScalaJSBundlerPlugin.scala#L790 and defined by the requireJsDomEnv boolean setting.

TBH I think that is a larger problem, that should be fixed first. It should be possible to just configure jsEnv := ... normally even when using ScalaJSBundler.

@armanbilge
Copy link

FYI, it's possible to use JSDOM with scoverage without this change. See scoverage/scalac-scoverage-plugin#456.

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

No branches or pull requests

4 participants