Add isNodeJs and process getters to cli_pkg/js.dart #143
Conversation
Goodwine
force-pushed
the
js-is-node
branch
from
b93d094 to
abd9488
Compare
Goodwine
force-pushed
the
js-is-node
branch
from
28803d3 to
f96010a
Compare
There was a problem hiding this comment.
When I suggested adding tests in browser_library_test, I meant updating the existing test files to verify that isNodeJs returns false for them. We specifically want to verify that their bundling strategies don't make it look like they're running under Node.js when we know they aren't.
There was a problem hiding this comment.
I don't think this should be in browser_library_test, since it's not running in a browser and it's not loading a JS library exposed by the package. It would probably make more sense to put it in test/node_js_test.dart.
There was a problem hiding this comment.
It is actually running in a browser 🤔, just without any other libraries involved. There are tests for running within Node already. I will rename this file to make it less confusing.
I guess it's a good idea to expand browser_test_shared.dart to verify none of those libraries would incorrectly return isNodeJs == true
Related to sass/dart-sass#2113