REPL module context failure

[thisolivier]

• Version: v9.1.0
• Platform: Darwin XXXX 17.2.0 Darwin Kernel Version 17.2.0: Fri Sep 29 18:27:05 PDT 2017; root:xnu-4570.20.62~3/RELEASE_X86_64 x86_64
• Subsystem: OSX 10.13.1

Using a fresh install of Node 9.1.0, with no global packages or local dependencies. I use NVM to manage node.

I am testing algorithms and trying out Node's new REPL module, attaching properties to the context at the end of my file as follows:

repl.start().context = {
    Bst: Bst,
    myBst: myBst
}

When I try to access either of these properties, I get an error:

TypeError: sandbox argument must have been converted to a context.
    at ContextifyScript.Script.runInContext (vm.js:59:29)
    at REPLServer.defaultEval (repl.js:244:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.onLine (repl.js:493:10)
    at emitOne (events.js:125:13)
    at REPLServer.emit (events.js:221:7)
    at REPLServer.Interface._onLine (readline.js:287:10)
    at REPLServer.Interface._line (readline.js:642:8)
    at REPLServer.Interface._ttyWrite (readline.js:922:14)

If I add just one property using dot notation repl.start().context.Bst = Bst then I can access that property. As an aside, I naively tried writing the following code with rather unexpected results:

repl.start().context.Bst = Bst
repl.start().context.myBst = myBst

If you run this code, two REPL servers will try and output to your shell at the same time, so every letter you type is typed twice, every command responded to (identically) twice. It's definitely two servers because incrementing a test variable will not increment it by 2, but display twice that the same variable has been incremented by 1 with the same result.

[thisolivier]

Further testing, the following code will run as expected (both properties can be accessed)

let context = repl.start().context
context.Bst = Bst
context.myBst = myBst

[bnoordhuis]

None of that is unexpected. The .context object is a bit magical, simply replacing it with a non-magical object won't work.

Calling repl.start() twice starts two instances. Not very useful perhaps but it did exactly what you asked it to.

[thisolivier]

@bnoordhuis - So the intended use is to start the server, and then add properties to the context post-hoc?
While I'm totally ok with this, there's a couple of points which seem odd, and I hope you'll indulge:

• Many frameworks allow you to define context as an object with properties. There should either be an update to the docs to avoid confusion, or a distinction between the getters and setters used here.
• The fact that I can start two competing servers which output to the same command line seems like a massive bug. Just typing is a janky fractured experience, and there's no notice for what's happening- if this is the intended functionality, it diverges significantly from anything else I've seen in Node. As an example, pressing 'm' on the keyboard outputs m m with a stutter.
• It was my understanding that adding properties to an object after it has been created is a large performance drain for Node's V8 engine, since many shadow classes have to be created, which makes the post-hoc process unscalable.

[bnoordhuis]

Pull request welcome if you think the documentation can be improved.

Hidden classes are the least of your concerns. VM context objects don't use them, they use something that is up to 200 times slower.

[thisolivier]

@bnoordhuis But to clarify (if I do update the docs), the intended use is to attach properties to the context post-hoc?

[bnoordhuis]

@thisolivier Correct.

[Trott]

It seems like perhaps this should be closed. Feel free to re-open (or leave a comment requesting that it be re-opened) if you disagree. I'm just tidying up and not acting on a super-strong opinion or anything like that.