repl: Default useGlobal to false in CLI REPL.

lance · Fishrock123 · commit 7a7b8f7e67de · 2016-07-05T22:36:52.000+02:00
Documentation for REPL states that the default value of `useGlobal` is `false`. It makes no distinction between a REPL that is created programmatically, and the one a user is dropped into on the command line by executing `node` with no arguments. This change ensures that the CLI REPL uses a default value of `false`. Fixes: #5659 Ref: #6802 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> PR-URL: #5703
diff --git a/lib/internal/repl.js b/lib/internal/repl.js
@@ -22,7 +22,7 @@ function createRepl(env, opts, cb) {
   opts = opts || {
     ignoreUndefined: false,
     terminal: process.stdout.isTTY,
-    useGlobal: true,
+    useGlobal: false,
     breakEvalOnSigint: true
   };
 
diff --git a/test/parallel/test-repl-use-global.js b/test/parallel/test-repl-use-global.js
@@ -0,0 +1,85 @@
+'use strict';
+
+// Flags: --expose-internals
+
+const common = require('../common');
+const stream = require('stream');
+const repl = require('internal/repl');
+const assert = require('assert');
+
+common.globalCheck = false;
+
+// Array of [useGlobal, expectedResult] pairs
+const globalTestCases = [
+  [false, 'undefined'],
+  [true, '\'tacos\''],
+  [undefined, 'undefined']
+];
+
+const globalTest = (useGlobal, cb, output) => (err, repl) => {
+  if (err)
+    return cb(err);
+
+  let str = '';
+  output.on('data', (data) => (str += data));
+  global.lunch = 'tacos';
+  repl.write('global.lunch;\n');
+  repl.close();
+  delete global.lunch;
+  cb(null, str.trim());
+};
+
+// Test how the global object behaves in each state for useGlobal
+for (const [option, expected] of globalTestCases) {
+  runRepl(option, globalTest, common.mustCall((err, output) => {
+    assert.ifError(err);
+    assert.strictEqual(output, expected);
+  }));
+}
+
+// Test how shadowing the process object via `let`
+// behaves in each useGlobal state. Note: we can't
+// actually test the state when useGlobal is true,
+// because the exception that's generated is caught
+// (see below), but errors are printed, and the test
+// suite is aware of it, causing a failure to be flagged.
+//
+const processTestCases = [false, undefined];
+const processTest = (useGlobal, cb, output) => (err, repl) => {
+  if (err)
+    return cb(err);
+
+  let str = '';
+  output.on('data', (data) => (str += data));
+
+  // if useGlobal is false, then `let process` should work
+  repl.write('let process;\n');
+  repl.write('21 * 2;\n');
+  repl.close();
+  cb(null, str.trim());
+};
+
+for (const option of processTestCases) {
+  runRepl(option, processTest, common.mustCall((err, output) => {
+    assert.ifError(err);
+    assert.strictEqual(output, 'undefined\n42');
+  }));
+}
+
+function runRepl(useGlobal, testFunc, cb) {
+  const inputStream = new stream.PassThrough();
+  const outputStream = new stream.PassThrough();
+  const opts = {
+    input: inputStream,
+    output: outputStream,
+    useGlobal: useGlobal,
+    useColors: false,
+    terminal: false,
+    prompt: ''
+  };
+
+  repl.createInternalRepl(
+      process.env,
+      opts,
+      testFunc(useGlobal, cb, opts.output));
+}
