Fix NODE_PATH for legacy projects

[wentout]

Seems this doesn't matter anymore
#132

But ths still relates to current state
clinicjs/node-clinic#148

So there might be changes done to fix NODE_PATH back, for legacy projects.

[goto-bus-stop]

hmm it looks like the PR that supports spaces in option values in Node was only landed 6 days ago as a semver major change: nodejs/node#24065

is the real problem here that we're overwriting NODE_PATH? we should do similar concatenation there as we're doing with NODE_OPTIONS, i think, so both user paths and our internal paths are supported.

[wentout]

Hi!

is the real problem here that we're overwriting NODE_PATH?

Yes, for me this was problem. Our legacy project is full of this NODE_PATH, and I was unable to run doctor on it. So the problem is overriding.

we should do similar concatenation there as we're doing with NODE_OPTIONS, i think, so both user paths and our internal paths are supported.

I think this is not about this PR, cause it dropps NODE_PATH overrdes.

[goto-bus-stop]

Right, but we need our custom NODE_PATH for file paths with spaces in them 😄 We can support both cases if we properly concatenate them. I believe it's something like this:

let nodePath = path.join(__dirname, 'injects')
if (process.env.NODE_PATH) {
  if (process.platform === 'win32') {
    nodePath += `;${process.env.NODE_PATH}`
  } else {
    nodePath += `:${process.env.NODE_PATH}`
  }
}

[wentout]

Right ... custom NODE_PATH .. something like this ...

@goto-bus-stop great thanks! It works!

Related PR for bubbleprof #319.

[wentout]

@goto-bus-stop it is strange, same changes for bubbleprof passed all tests.
And this test fails for sampling interval checker. Moreover, it has unstable behaviour, it fails only on Mac bundles and may fail or not. As I logged there for

test('cmd - collect - data files have content ...

seems checking on direct value of not more than 20ms is not so stable check and may depend on something else there, in a cloud.

[goto-bus-stop]

Yeah sorry, that test is flaky. You can ignore those failures. I have a PR to loosen the check in #237. Could you revert the changes to the test? Rest LGTM :D

[wentout]

Yeah sorry, that test is flaky. You can ignore those failures. I have a PR to loosen the check in #237. Could you revert the changes to the test? Rest LGTM :D

I did, thank you for explanations ^)

[goto-bus-stop]

Published as 📦 4.0.4
Thanks!