I previously had thought that a duplicate call to `setRequireAndModule`
encountered in code-path would no longer be necessary after some
consolidation in previous steps of this re-factor, but the test failure
seen here made it clear what was happening:
https://circleci.com/gh/meteor/meteor/12445
Specifically, if a module was imported in a piped command (that is to say,
when no TTY is present and the `evaluateAndExit` code-path is taken), as so:
echo 'import { Meteor } from "meteor/meteor"' | meteor shell
...the `module` and `require` symbols were not set. Conveniently, this is
the environment in effect when the `meteor self-test` suite is ran since
they do not have a TTY.
This moves the `setAndRequire` from the "interactive-only" function into
the general REPL setup and further harmonizes the code.
This is superfluous residue that I inadvertently created when splitting the
existing `startREPL` function into `setupREPL` and `enableInteractiveMode`.
The context is already set in `setupREPL` (to the exact same value as
here) by the time that this occurrence in `enableInteractiveMode` is called.
Addresses feedback from @benjamn.
Rather than copying the `IsRecoverableError` and `isCodeRecoverable`
methods from the Node.js `repl` module source (in order to capture
so-called "Recoverable" errors), wrap the default "eval" function with
our relatively thin logic, thus avoiding the need to continually update
the definition of what's "recoverable" as Node's implementation evolves.
A truthy `useGlobal` option when calling Node's `repl.start()` will use
the `global` context, which will allow global Meteor variables (such as
the global `_` provided by the `underscore`) to be available in the context
of the REPL shell. This sounds desirable, and in fact, the original
implementation set it as such, only to be later changed to `false` in
https://github.com/meteor/meteor/commit/7c7e52f2d2, specifically to
avoid the undesirable behavior of Node REPL stomping on the global `_`
variable with its own special-meaning `_` variable which is set to the
value of the most recently eval'd expression.
This was once again changed to `true` to fix the lost tab-completion
functionality, thanks to https://github.com/meteor/meteor/commit/2443d83226,
which also implelented special mutator on `_` to avoid breaking
it. As it's probably clear now, this has been a struggle, and because
of Node using its own `Object.defineProperty('_', ...)` as of Node.js 6
(in https://github.com/nodejs/node/pull/5535/files), this problem has
surfaced again, as reported in https://github.com/meteor/meteor/issues/9276.
This changes the behavior to another implementation which starts with
`useGlobal` set to `false`, but then sets the context immediately to
`global`, which seems to avoid the problem.
It's possible that another option might be to set explicitly set the
`underscoreAssigned` attribute on the `repl` after starting it, however
since that's an internal, undocumented property, it might be best to avoid.
This does maintain the existing behavior of our special
double-underscore variable, in order to get the value of the last
REPL expression, in the same manner as `_` would in a normal REPL, by
accessing the internal `last` property. Since this is also undocumented
(on our end), this seemed reasonable.
In order to catch so-called "recoverable" REPL errors (for example,
commands in the process of being inputted which are syntax errors until
they are completed/recovered), it had previously been necessary to
intentionally cause such an error and save the error's prototype for
future use.
Node 6 changed this by exporting the Error directly from the `repl`
module, and this commit represents the changes necessary to remove that
(now unnecessary) behavior.
This also takes the opportunity to update the `isRecoverableError` (and
related) functions which had previously been borrowed from the `repl`
module source, to their newer versions.
