add extended Node feature check to fix issue #8816 - #8832
Conversation
| // This will allow the former to do things like mount NODEFS. | ||
| ENVIRONMENT_HAS_NODE = typeof process === 'object' && typeof require === 'function'; | ||
| // Extended check using process.versions added to fix issue #8816. | ||
| ENVIRONMENT_HAS_NODE = typeof process === 'object' && typeof process.versions === 'object' && typeof process.versions.node === "string" && typeof require === 'function'; |
There was a problem hiding this comment.
Do we need the check for require? I think given we now check explicitly for node, it is redundant?
|
It might be redundant, but I left it in to document the fact that the canonical heuristic check for Node (according to The Google) utilizes process and require, and we are simply adding to that canonical check ... might be useful to remember when the next exception case rears its head. |
|
@kripken ummm ... were you actually asking me to remove the check? How can I help move this along? |
|
Sorry for the delay. Overall I think we should remove it if it's unnecessary - can mention in a comment, if you think the information is useful? That way we don't pay for it in code size in minified builds. |
|
I removed the redundant check on 'require', tested on Electron (with and without Node) and on Web, and (I believe) GitHub has merged the new commit, so we should be all set ... |
|
Great, thanks @ericmandel! |
2 participants
Thank you for submitting a pull request!
If this is your first PR, make sure to add yourself to AUTHORS.