Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixed Node-detection #25

Merged
merged 1 commit into from
Mar 5, 2016
Merged

fixed Node-detection #25

merged 1 commit into from
Mar 5, 2016

Conversation

florianb
Copy link
Contributor

@florianb florianb commented Mar 5, 2016

The previous implementation was depending on the window-property which is at least in the Electron-environment available although the javascript is running in a Node-process.

The new detection-statement relies on the process.release-object which allows the safe identification of node.js and io.js.

P.s.: I didn't open an issue since it seems to be an atomic change. I hope filing immediately a PR is not considered as rude behaviour.

馃嵒

The previous implementation was depending on the window-property which is at least in the Electron-environment available although the javascript is running in a Node-process.

The new detection-statement relies on the [`process.release`](https://nodejs.org/api/process.html#process_process_release)-object which allows the safe identification of node.js and io.js.
@asvd
Copy link
Owner

asvd commented Mar 5, 2016

Good point, thanks! I did not investigate much on proper detection before, and just picked the first solution that worked.

(I'm working on the project by myself, so there's not that much rules 馃槃)

asvd added a commit that referenced this pull request Mar 5, 2016
@asvd asvd merged commit 659788a into asvd:master Mar 5, 2016
@florianb
Copy link
Contributor Author

florianb commented Mar 7, 2016

No worries - i'm happy you created and offer jailed! 馃槂

(And if i get a clue to improve something you'll receive another PR.. 馃槈)

@asvd
Copy link
Owner

asvd commented Mar 7, 2016

sure, you are welcome! 馃憤 (but I don't accept everything just like that 馃槈)

mickeyh pushed a commit to mickeyh/jailed that referenced this pull request Jun 13, 2016
`__jailed__path__` is defined depending on whether the environment is
Node or a browser. The earlier node detection fix was not applied to the
code that controlled how `__jailed__path__` was populated.

See asvd#25 for original node detection fix.
mickeyh pushed a commit to mickeyh/jailed that referenced this pull request Jun 13, 2016
`__jailed__path__` is defined depending on whether the environment is
Node or a browser. The earlier node detection fix was not applied to the
code that controlled how `__jailed__path__` was populated.

See asvd#25 for original node detection fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants