REPL: _ and _error variables are not available by default in a repl instance with a modified context

[sjsyrek]

• Version: 15.1.0
• Platform: 19.6.0 Darwin Kernel Version 19.6.0: Mon Aug 31 22:12:52 PDT 2020; root:xnu-6153.141.2~1/RELEASE_X86_64 x86_64
• Subsystem: repl

What steps will reproduce the bug?

Start an application that imports repl and modifies the context. For example: https://github.com/sjsyrek/malcjs

Update: I applied a workaround to this repo (see below), but if the lines starting at https://github.com/sjsyrek/malcjs/blob/master/src/index.js#L67 are commented out, the issue can still be reproduced.

Basically,

const context = {
  // whatever
};

vm.createContext(context);

const replServer = repl.start();

replServer.context = context;

Try to use the _ or _error variables for previously evaluated expressions. They do not work as expected.

How often does it reproduce? Is there a required condition?

Every time.

What is the expected behavior?

https://nodejs.org/api/repl.html#repl_assignment_of_the_underscore_variable

I expect that when I create my own context for a custom REPL, it won't affect any other initialization steps, including the availability of the (extremely useful) _ and _error variables. I cannot say for sure that anything else is missed in this scenario, but it would be worth investigating (example, entering util.inspect.replDefaults in a custom REPL results in an error, whereas the standard REPL returns the inspection defaults).

What do you see instead?

I have observed the strange behavior that when I enter _ at the repl prompt, it gets duplicated, i.e. __ is entered instead and I see this error:

Uncaught ReferenceError: __ is not defined

Additional information

It seems that I was able to resolve this issue by adapting code directly from the repl package and defining these variables myself, e.g.:

Object.defineProperty(replServer.context, "_", {
  configurable: true,
  get: () => replServer.last,
  set: (value) => {
    replServer.last = value;
    if (!replServer.underscoreAssigned) {
      replServer.underscoreAssigned = true;
      replServer.output.write("Expression assignment to _ now disabled.\n");
    }
  },
});

Object.defineProperty(replServer.context, "_error", {
  configurable: true,
  get: () => replServer.lastError,
  set: (value) => {
    replServer.lastError = value;
    if (!replServer.underscoreErrAssigned) {
      replServer.underscoreErrAssigned = true;
      replServer.output.write(
        "Expression assignment to _error now disabled.\n"
      );
    }
  },
});

But I assume this is not desirable.

[sjsyrek]

I may have jumped the gun. It seems you can't simply assign a new context and expect normal behavior, you need to assign properties to the context one at a time. So I did this instead:

for (const property in context) {
  replServer.context[property] = context[property];
}

If this is the expected and desired behavior, vs. creating an object, initializing it as a context with vm.createContext, and assigning that to the REPL instance, then I suppose this issue could be closed. But it seems like this is an inconvenient way to go about it.

[targos]

I think the best solution is to let Node.js create the new context for you and then assign your extensions to it with Object.assign(repl.context, { ... })

[sjsyrek]

@targos Great idea, thanks. Is this worth documenting?

[BridgeAR]

Adding properties to the existing context is actually not sufficient. A user could use the regular commands to clear the context at any time (e.g., call .clear). That way a new context is created and the individual context is lost. At least that command would have to manually be removed. I can not tell how frequent such use case is and if we should try to officially support changing the context. If so, passing through an individual context should likely prohibit resetting that context again or we allow passing through a createContext function that is called in such case.