doc: vm, run http server in vm by requiring#5323
Uh oh!
There was an error while loading. Please reload this page.
Conversation
doc/api/vm.markdown Outdated
There was a problem hiding this comment.
Please make valuable comments that bring the discussion forth not back. Any other way then than this and eval?
There was a problem hiding this comment.
vm.runInThisContext(`(function(require){ require('http')})`)(require)
eljefedelrodeodeljefe commented Feb 19, 2016
Force pushed this. Doesn't look pretty but solves the problem. |
mscdex added doc 
Knighton910 commented Feb 19, 2016
doc/api/vm.markdown Outdated
There was a problem hiding this comment.
minor nit... having the let code = on a separate line with no indenting on the next line makes it a bit confusing.
There was a problem hiding this comment.
agree. fixed and force pushed. Thx.
jasnell commented Feb 19, 2016
LGTM |
doc/api/vm.markdown Outdated
There was a problem hiding this comment.
I think this should either not be a heading, or a 3rd level heading.
There was a problem hiding this comment.
I think we are using the heading style when it is an example, outside of a method, but rather a guide or such. E.g. here https://github.com/nodejs/node/blob/master/doc/api/readline.markdown#example-tiny-cli . If you'd rather want it belonging to .runInThisContext(), I can do it w/o pounds.
Having a look at for example streams, it would be 3rd level, but because it's having 2nd level topics like Writable etc. We are a little inconsistent about this I fear. :(
No strong opinions though.
eljefedelrodeodeljefe commented Mar 7, 2016
@jasnell agree. Changed it to what you provided. Also added this issue, for potential harmonization of pronouns nodejs/docs#87 |
Qard commented Mar 7, 2016
Not sure how I feel about documenting this in this way. Normally the vm functions are meant to provide a sandboxed context and this is doing the opposite. The problem I see here is that it's not immediately clear that require is not just a stateless function. Using require will stuff modules into the cache globally, and you could do some nefarious things, like mess with the contents of As a side note: I'd only really consider |
eljefedelrodeodeljefe commented Mar 8, 2016
I'd wish for the API to be a little less confusing, but it is what is and at least it allows for some configuration. Also I wouldn't use Documenting dangers would be fine of course. Maybe also hinting to the use case above. |
jasnell commented Mar 21, 2016
@Qard@eljefedelrodeodeljefe ... any further thoughts on this one? |
Qard commented Mar 22, 2016
It worries me a little, but I think I'm 👍 overall. LGTM. Might be worth discussing more in a docs meeting, but I think we can add to it later rather than blocking. |
MylesBorins commented Mar 23, 2016
@eljefedelrodeodeljefe I was just about to land this but noticed the commit message is not so informative. Would you be able to update that? LGTM otherwise |
eljefedelrodeodeljefe commented Mar 30, 2016
I have updated the commit message to reflect intention. Also I have added @Qard's remarks concerning the statefulness of require in this case in a note-section below the example. Also happy in case we need broader discussion |
jasnell commented Apr 1, 2016
LGTM |
doc/api/vm.markdown Outdated
There was a problem hiding this comment.
Minot nit. I think it is V8.
eljefedelrodeodeljefe commented Apr 2, 2016
Addressed latest comments and force pushed. @thefourtheye now with semi-colons, thx. |
jasnell commented Apr 19, 2016
ping @thefourtheye |
From squashed: remove you reference, @jasnell Also: The intention behind is to present the user a way he/she can execute the node code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Proposed API changes, haven't moved forward. ref: nodejs/node-v0.x-archive#9211, #4955
eljefedelrodeodeljefe commented Apr 21, 2016
rebased. Also tried to contact @thefourtheye concerning sign-off on slack. But I think he is just incredibly busy. |
thefourtheye commented Apr 21, 2016
Really sorry about the delay. LGTM. |
jasnell commented Apr 21, 2016
Awesome, ok, one final ping to @nodejs/documentation as a last call for comments. |
benjamingr commented Apr 21, 2016
LGTM |
1 similar comment
Qard commented Apr 21, 2016
LGTM |
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
jasnell commented Apr 22, 2016
Landed in 6815a3b. Fixed up the commit log a bit |
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, nodejs#4955 PR-URL: nodejs#5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
MylesBorins commented Jun 1, 2016
@jasnell / @eljefedelrodeodeljefe is this something we want for lts? |
MylesBorins commented Jul 1, 2016
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
The intention behind is to present the user a way to execute code in a vm context. The current API doesn't allow this out-of-the-box, since it is neither passing a require function nor creating context with one. The missing docs for this behaviour have produced a number of Q&A items and have also been discussed in the node-archive repo. In both cases there was no real canonical answer. Refs: nodejs/node-v0.x-archive#9211, #4955 PR-URL: #5323 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
11 participants
As user in order to run
nodecode in avm, you would need to apply some objects to it. This is not quite well documented and may or may not have been reason for a series of issues and SO questions.This PR would add an example to explain this. It is to some extent controversial and was discussed in a code PR #4955. Issue would have been nodejs/node-v0.x-archive#9211 and this link to SO.
An easier solution would be to use
eval(), which I regard as an anti-pattern.Also it usesI have taken and modified this example from the referenced issues.require('module').wrap(code), which I believe to be a "private" API./cc @nodejs/documentation