Support `forkWorkingDir` in `ScalaJSModule` (`run`, `test`, ...)

[lolgab]

Working with multiple node.js subprojects, it is needed to have different installation directories that carry their own node_modules folder. Currently it is not possible to change set the pwd for a subproject, so tests always run by using the root project directory as pwd. This is probably something that can be useful on a more general level than the ScalaJSModule since it can be useful for Scala and Java projects too.

[lefou]

Mill supports changing the working directory for forked tests and in general forked runs. This is controlled via the JavaModule.forkWorkingDir target. But it looks like (as you have noted yourself on gitter channel) that this is not used in ScalaJSModule to run tests.

[nafg]

I can send a PR if I'm told how to do it

[lefou]

I think, you need to add a working dir parameter to ScalaJSWorker.run method and plumb it into the right tools invocations in the implementation. Once, that is done, all usages of the worker in ScalaJSModule need to use the forkWorkingDir target to feed that parameter. This is the run target as well as the test target in TestScalaJSModule, and maybe others.

[nafg]

I started writing a longer comment but tldr this requires a breaking change to scalajs-js-envs.

But I just realized that it may not be necessary for my use case. The original problem was how to get Node to use a generated node_modules somewhere under out/. And I had tried using the NODE_PATH environment variable and it didn't seem to work, and AFAICT nobody on the internet uses it this way, and not being able to set the working directory seems like an arbitrary limitation that I assumed would easy to fix, so I started thinking the solution was in that direction.

But I realized that I was probably doing it wrong, setting it to the directory containing node_modules. I think instead it should point to the actual node_modules, as implied in https://nodejs.org/api/modules.html#loading-from-the-global-folders.

So maybe I can get ScalaJS tests to run without bundling that way.

[lefou]

I started writing a longer comment but tldr this requires a breaking change to scalajs-js-envs.

Can you elaborate on this? Why needs it a change in an upstream dependency? This issue is about defining the working (or start) directory of the node.js process. Everything else seems unrelated to this issue.

But I just realized that it may not be necessary for my use case. The original problem was how to get Node to use a generated node_modules somewhere under out/. And I had tried using the NODE_PATH environment variable and it didn't seem to work, and AFAICT nobody on the internet uses it this way, and not being able to set the working directory seems like an arbitrary limitation that I assumed would easy to fix, so I started thinking the solution was in that direction.

But I realized that I was probably doing it wrong, setting it to the directory containing node_modules. I think instead it should point to the actual node_modules, as implied in https://nodejs.org/api/modules.html#loading-from-the-global-folders.

So maybe I can get ScalaJS tests to run without bundling that way.

The motivation is to have two modules which for whatever reasons should not share the same node.js setup. So instead of the project root directory (T.workspace) it could be the module directory (millSourcePath) or also something under out (as you, @nafg, initially wanted).

[nafg]

Can you elaborate on this? Why needs it a change in an upstream dependency? This issue is about defining the working (or start) directory of the node.js process. Everything else seems unrelated to this issue.

Because Mill doesn't actually call Node itself. That's the responsibility of the JSEnv.

Here is some of the longer reply I was writing:

Ultimately ScalaJSWorker.run is calling https://github.com/nafg/mill/blob/5c48ba4121a129d5e8ec1271f462f0e51e314c97/scalajslib/worker/1/src/Run.scala#L21 which calls org.scalajs.jsenv.JSEnv#start which is not part of Mill. In any case the behavior depends on the particular subclass of JSEnv, but at any rate there's no API to pass a path in at this level. But all of them currently known by Mill delegate to org.scalajs.jsenv.ExternalJSRun.start, which calls startProcess which uses java.lang.ProcessBuilder and does not set its directory.

One way to pass it in is by adding it as a field to org.scalajs.jsenv.RunConfig. But in any case it's not possible to add it anywhere AFAICT without a binary incompatible change to scalajs-js-envs which I'm guessing @sjrd isn't going to be super excited to do.

[nafg]

I started writing a longer comment but tldr this requires a breaking change to scalajs-js-envs.

But I just realized that it may not be necessary for my use case. The original problem was how to get Node to use a generated node_modules somewhere under out/. And I had tried using the NODE_PATH environment variable and it didn't seem to work, and AFAICT nobody on the internet uses it this way, and not being able to set the working directory seems like an arbitrary limitation that I assumed would easy to fix, so I started thinking the solution was in that direction.

But I realized that I was probably doing it wrong, setting it to the directory containing node_modules. I think instead it should point to the actual node_modules, as implied in https://nodejs.org/api/modules.html#loading-from-the-global-folders.

So maybe I can get ScalaJS tests to run without bundling that way.

That seems to have worked! https://github.com/nafg/mill-bundler/pull/2

[lefou]

Can you elaborate on this? Why needs it a change in an upstream dependency? This issue is about defining the working (or start) directory of the node.js process. Everything else seems unrelated to this issue.

Because Mill doesn't actually call Node itself. That's the responsibility of the JSEnv.

Here is some of the longer reply I was writing:

Ultimately ScalaJSWorker.run is calling https://github.com/nafg/mill/blob/5c48ba4121a129d5e8ec1271f462f0e51e314c97/scalajslib/worker/1/src/Run.scala#L21 which calls org.scalajs.jsenv.JSEnv#start which is not part of Mill. In any case the behavior depends on the particular subclass of JSEnv, but at any rate there's no API to pass a path in at this level. But all of them currently known by Mill delegate to org.scalajs.jsenv.ExternalJSRun.start, which calls startProcess which uses java.lang.ProcessBuilder and does not set its directory.

One way to pass it in is by adding it as a field to org.scalajs.jsenv.RunConfig. But in any case it's not possible to add it anywhere AFAICT without a binary incompatible change to scalajs-js-envs which I'm guessing @sjrd isn't going to be super excited to do.

Interesting. As JsEnv seem to be design around the idea to keep some "global state" of the JS environment in the current running JVM, we probably need to spawn a new JVM process from the new working directory, and call the JsEnv API from this process.