Fix initialization of nodePath variable

[jrprice]

When running Emscripten-generated scripts via node -e "<script>",
the fs variable is already defined and so nodePath was never being
initialized.

This fixes a breakage caused by #15587.

[kripken]

Oh, wow, is the issue that node will create a global fs when you use node -e CODE as opposed to node FILE where FILE is a file containing CODE? I tested that now and looks like it's the case...

How do you hit this with an emscripten program, btw? Are you doing something like node -e "require('emscripten-ouput.js')"? (otherwise it doesn't seem likely for emscripten JS to appear in node -e. But then why not write node emscripten-output.js which will not hit the problem?

This PR looks reasonable. But IIUC it only works because we have fs = require("fs") and that value is exactly the same as what node assigns to fs itself, which seems a little fragile. Another option here might be to not use fs, but maybe nfs ("node fs"). That would be a larger change, though, and I'm not sure it's worth it.

Please add a comment in the code that explains why we need that extra check.

A small test in tests/test_other.py would be good for this.

[jrprice]

How do you hit this with an emscripten program, btw? Are you doing something like node -e "require('emscripten-ouput.js')"? (otherwise it doesn't seem likely for emscripten JS to appear in node -e. But then why not write node emscripten-output.js which will not hit the problem?

Yeah, actually it was node -e "<some extra setup stuff that I need>; $(cat emscripten-output.js)"

But using require("emscripten-output.js") seems to work fine, and somebody else pointed out that I could also use -r to preload a module. Still, I assume it's nice to have this work for my weird node -e "$(cat blah.js) case above? If not, feel free to close this since I have usable workarounds now.

This PR looks reasonable. But IIUC it only works because we have fs = require("fs") and that value is exactly the same as what node assigns to fs itself, which seems a little fragile. Another option here might be to not use fs, but maybe nfs ("node fs"). That would be a larger change, though, and I'm not sure it's worth it.

Yeah, this variable used to be called something different prior to #15587 , which is why this has only just come up.

Please add a comment in the code that explains why we need that extra check.

Done.

A small test in tests/test_other.py would be good for this.

Done, hopefully it's not too fragile.

[sbc100]

Is node -e "<some extra setup stuff that I need>; $(cat emscripten-output.js)" really something that is done? Wouldn't that end with a command line that was tens of thousands of chars long on most cases? I guess there is no harm in supporting but it seems like an odd way to run a program.

[jrprice]

Is node -e "<some extra setup stuff that I need>; $(cat emscripten-output.js)" really something that is done? Wouldn't that end with a command line that was tens of thousands of chars long on most cases? I guess there is no harm in supporting but it seems like an odd way to run a program.

Agreed. I did it because I didn't know about -r or require(), but anyone who actually knows stuff about JS and node would probably not do this.

[merceyz]

It's the same with the REPL so if you're testing stuff there you'll run into it as well.

[sbc100]

Would you mind rebasing or merging with origin/main? It should fix the failures.