Separate prefix nums in eval contexts [fix]

Fixes #538.

lib/init/eval.js

@@ -11,7 +11,9 @@ const generate = require('@babel/generator').default,
 	t = require('@babel/types');
 
 // Imports
-const {parseImpl} = require('../instrument/instrument.js'),
+const createTracker = require('./tracker.js'),
+	getScopeId = require('./getScopeId.js'),
+	{parseImpl} = require('../instrument/instrument.js'),
 	modifyAst = require('../instrument/modify.js'),
 	{
 		createBlockWithId, createThisBinding, createNewTargetBinding, createBindingWithoutNameCheck
@@ -20,7 +22,6 @@ const {parseImpl} = require('../instrument/instrument.js'),
 		INTERNAL_VAR_NAMES_PREFIX, TRACKER_VAR_NAME_BODY, GET_SCOPE_ID_VAR_NAME_BODY
 	} = require('../shared/constants.js'),
 	specialFunctions = require('../shared/internal.js').functions,
-	getScopeId = require('./getScopeId.js'),
 	assertBug = require('../shared/assertBug.js');
 
 // Constants
@@ -32,15 +33,19 @@ const DEBUG = !!process.env.LIVEPACK_DEBUG_INSTRUMENT;
  * Add eval methods to tracker.
  * @param {Function} tracker - Tracker function for file
  * @param {string} filename - File path
+ * @param {Object} blockIdCounter - Block ID counter for file
+ * @param {number} prefixNum - Internal vars prefix num
  * @returns {undefined}
  */
-module.exports = function addEvalFunctionsToTracker(tracker, filename) {
+module.exports = function addEvalFunctionsToTracker(tracker, filename, blockIdCounter, prefixNum) {
 	const evalIndirectLocal = {
 		eval(code) {
-			return evalIndirect(code, tracker, filename, evalIndirectLocal);
+			return evalIndirect(code, tracker, filename, blockIdCounter, prefixNum, evalIndirectLocal);
 		}
 	}.eval;
-	const evalDirectLocal = (...args) => evalDirect(args, tracker, filename, evalDirectLocal);
+	const evalDirectLocal = (...args) => evalDirect(
+		args, tracker, filename, blockIdCounter, prefixNum, evalDirectLocal
+	);
 	tracker.evalIndirect = evalIndirectLocal;
 	tracker.evalDirect = evalDirectLocal;
 
@@ -55,17 +60,26 @@ module.exports = function addEvalFunctionsToTracker(tracker, filename) {
  * @param {*} code - Argument to `eval`
  * @param {Function} tracker - Tracker function for file
  * @param {string} filename - File path
+ * @param {Object} blockIdCounter - Block ID counter for file
+ * @param {number} externalPrefixNum - Internal vars prefix num outside `eval`
  * @param {Function} evalIndirectLocal - Function which was called (used for stack traces if error)
  * @returns {*} - Result of `eval()` call
  */
-function evalIndirect(code, tracker, filename, evalIndirectLocal) {
+function evalIndirect(code, tracker, filename, blockIdCounter, externalPrefixNum, evalIndirectLocal) {
 	// If `code` arg is not a string, eval it unchanged - it won't be evaluated as code
 	// eslint-disable-next-line no-eval
 	if (!isString(code)) return execEvalCode(eval, code, false, code, evalIndirectLocal);
 
 	// Compile code with no external scope.
 	// Code returned is for a function which takes arguments `(livepack_tracker, livepack_getScopeId)`.
-	const {code: fnCode, shouldThrow} = compile(code, tracker, filename, true, undefined, false);
+	const {code: fnCode, shouldThrow, internalPrefixNum} = compile(
+		code, filename, blockIdCounter, externalPrefixNum, true, undefined, false
+	);
+
+	// If prefix num inside `eval` is different from outside, create new tracker
+	if (internalPrefixNum !== externalPrefixNum) {
+		tracker = createTracker(filename, blockIdCounter, internalPrefixNum);
+	}
 
 	// `eval()` code without external scope and inject in Livepack's vars
 	// eslint-disable-next-line no-eval
@@ -81,10 +95,12 @@ function evalIndirect(code, tracker, filename, evalIndirectLocal) {
  * @param {Array<*>} args - Arguments `eval()` called with
  * @param {Function} tracker - Tracker function for file
  * @param {string} filename - File path
+ * @param {Object} blockIdCounter - Block ID counter for file
+ * @param {number} externalPrefixNum - Internal vars prefix num outside `eval`
  * @param {Function} evalDirectLocal - Function which was called (used for stack traces if error)
  * @returns {*} - Result of `eval()` call
  */
-function evalDirect(args, tracker, filename, evalDirectLocal) {
+function evalDirect(args, tracker, filename, blockIdCounter, externalPrefixNum, evalDirectLocal) {
 	const callArgs = args.slice(0, -5),
 		code = callArgs[0],
 		[possibleEval, execEvalSingleArg, execEvalSpread, scopeDefs, isStrict] = args.slice(-5);
@@ -131,18 +147,20 @@ function evalDirect(args, tracker, filename, evalDirectLocal) {
 
 	// Compile to executable code with tracking code inserted.
 	// If var names prefix inside code has to be different from outside,
-	// code is wrapped in an IIFE which renames the tracker/eval functions:
-	// `() => foo` ->
-	// `((livepack1_tracker, livepack1_getScopeId) => () => foo)(livepack_tracker, livepack_getScopeId)`
+	// code is wrapped in a function which injects tracker and getScopeId functions:
+	// `() => foo` -> `(livepack1_tracker, livepack1_getScopeId) => () => foo`
 	const {
-		code: codeInstrumented, shouldThrow, prefixNumChanged
-	} = compile(code, tracker, filename, false, state, isStrict);
+		code: codeInstrumented, shouldThrow, internalPrefixNum
+	} = compile(code, filename, blockIdCounter, externalPrefixNum, false, state, isStrict);
 
 	// Call `eval()` with amended code
 	let res = execEvalCode(execEvalSingleArg, codeInstrumented, shouldThrow, code, evalDirectLocal);
 
-	// If was wrapped in a function, inject values into function
-	if (prefixNumChanged) res = res(tracker, getScopeId);
+	// If was wrapped in a function, create new tracker and inject tracker and `getScopeId` into function
+	if (internalPrefixNum !== externalPrefixNum) {
+		tracker = createTracker(filename, blockIdCounter, internalPrefixNum);
+		res = res(tracker, getScopeId);
+	}
 
 	return res;
 }
@@ -185,18 +203,19 @@ function execEvalCode(exec, arg, shouldThrowSyntaxError, code, fn) {
  * `((livepack20_tracker, livepack20_getScopeId) => { ... })(livepack_tracker, livepack_getScopeId)`
  *
  * @param {string} code - Code string passed to `eval()`
- * @param {Function} tracker - Tracker function for file
  * @param {string} filename - File path
+ * @param {Object} blockIdCounter - Block ID counter for file
+ * @param {number} externalPrefixNum - Internal vars prefix num outside `eval()`
  * @param {boolean} isIndirectEval - `true` if is indirect eval
  * @param {Object} [state] - State to initialize instrumentation state (only if direct `eval()` call)
  * @param {boolean} isStrict - `true` if environment outside `eval()` is strict mode
  * @returns {Object} - Object with properties:
  *   {string} .code - Instrumented code (or input code if parsing failed)
  *   {boolean} .shouldThrow - `true` if could not parse code, so calling `eval()` with this code
  *     should throw syntax error
- *   {boolean} .prefixNumChanged - `true` if prefix num inside `eval()` differs from outside
+ *   {number} .internalPrefixNum - Internal vars prefix num inside `eval()`
  */
-function compile(code, tracker, filename, isIndirectEval, state, isStrict) {
+function compile(code, filename, blockIdCounter, externalPrefixNum, isIndirectEval, state, isStrict) {
 	// Parse code.
 	// If parsing fails, swallow error. Expression will be passed to `eval()`
 	// which should throw - this maintains native errors.
@@ -206,7 +225,7 @@ function compile(code, tracker, filename, isIndirectEval, state, isStrict) {
 			code, filename, false, false, !isIndirectEval, false, isStrict, false, undefined
 		).ast;
 	} catch (err) {
-		return {code, shouldThrow: true, prefixNumChanged: false};
+		return {code, shouldThrow: true, internalPrefixNum: externalPrefixNum};
 	}
 
 	// Instrument code.
@@ -215,25 +234,25 @@ function compile(code, tracker, filename, isIndirectEval, state, isStrict) {
 	// Var name prefix will be kept same as in host file if possible,
 	// to avoid wrapping in a function unless impossible to avoid.
 	// Details of vars which can be obtained from external scopes is passed in.
-	const externalPrefixNum = tracker.prefixNum;
 	state = {
-		nextBlockId: tracker.nextBlockId,
+		nextBlockId: blockIdCounter.nextBlockId,
 		isStrict,
 		internalVarsPrefixNum: externalPrefixNum,
 		...state
 	};
 	modifyAst(ast, filename, false, isStrict, undefined, state);
 
-	// Update next block ID and prefix num for file
-	tracker.nextBlockId = state.nextBlockId;
-	const internalPrefixNum = tracker.prefixNum = state.internalVarsPrefixNum;
+	// Update next block ID for file
+	blockIdCounter.nextBlockId = state.nextBlockId;
 
 	// If indirect `eval`, or direct `eval()` with different prefix nums inside and outside `eval()`,
 	// wrap in function to inject Livepack's internal vars.
 	// `123` => `(livepack_tracker, livepack_getScopeId) => eval('123')`.
 	// Wrapping in a 2nd `eval()` is required to ensure it returns its value.
-	const prefixNumChanged = internalPrefixNum !== externalPrefixNum;
-	if (isIndirectEval || prefixNumChanged) ast = wrapInFunction(ast, internalPrefixNum);
+	const internalPrefixNum = state.internalVarsPrefixNum;
+	if (isIndirectEval || internalPrefixNum !== externalPrefixNum) {
+		ast = wrapInFunction(ast, internalPrefixNum);
+	}
 
 	// Return instrumented code
 	code = generate(ast, {retainLines: true, compact: true}).code;
@@ -248,7 +267,7 @@ function compile(code, tracker, filename, isIndirectEval, state, isStrict) {
 		/* eslint-enable no-console */
 	}
 
-	return {code, shouldThrow: false, prefixNumChanged};
+	return {code, shouldThrow: false, internalPrefixNum};
 }
 
 function wrapInFunction(ast, internalPrefixNum) {

lib/init/index.js

@@ -8,10 +8,9 @@
 'use strict';
 
 // Imports
-const getScopeId = require('./getScopeId.js'),
-	addEvalFunctionsToTracker = require('./eval.js'),
+const createTracker = require('./tracker.js'),
+	getScopeId = require('./getScopeId.js'),
 	internal = require('../shared/internal.js'),
-	{tracker} = require('../shared/tracker.js'),
 	{COMMON_JS_MODULE} = require('../shared/constants.js');
 
 // Exports
@@ -39,10 +38,8 @@ module.exports = (filename, module, require, nextBlockId, prefixNum) => {
 	specialFunctions.set(require, {type: 'require', path: filename});
 
 	// Create local tracker function with additional properties and methods specific to the file
-	const localTracker = (getFnInfo, getScopes) => tracker(getFnInfo, getScopes);
-	localTracker.nextBlockId = nextBlockId;
-	localTracker.prefixNum = prefixNum;
-	addEvalFunctionsToTracker(localTracker, filename);
+	const blockIdCounter = {nextBlockId};
+	const localTracker = createTracker(filename, blockIdCounter, prefixNum);
 
 	// Return tracker and `getScopeId` functions
 	return [localTracker, getScopeId];

lib/init/tracker.js

@@ -0,0 +1,29 @@
+/* --------------------
+ * livepack module
+ * Init.
+ * Create tracker function for file or for `eval()`.
+ * ------------------*/
+
+'use strict';
+
+// Export before imports to avoid circular require with `./eval.js`
+module.exports = createTracker;
+
+// Imports
+const addEvalFunctionsToTracker = require('./eval.js'),
+	{tracker} = require('../shared/tracker.js');
+
+// Exports
+
+/**
+ * Create local tracker function with additional properties and methods specific to the file.
+ * @param {string} filename - File path
+ * @param {Object} blockIdCounter - Block ID counter for file
+ * @param {number} prefixNum - Internal vars prefix num
+ * @returns {Function} - Local tracker function
+ */
+function createTracker(filename, blockIdCounter, prefixNum) {
+	const localTracker = (getFnInfo, getScopes) => tracker(getFnInfo, getScopes);
+	addEvalFunctionsToTracker(localTracker, filename, blockIdCounter, prefixNum);
+	return localTracker;
+}

test/eval.test.js

@@ -1730,4 +1730,54 @@ describe('eval', () => {
 			}
 		});
 	});
+
+	describe('multiple `eval()`s evaluated before serialization', () => {
+		// Tests cover https://github.com/overlookmotel/livepack/issues/538
+		itSerializesEqual('simple case', {
+			in: `
+				'use strict';
+				const x = 11, y = 22;
+				module.exports = [eval('x'), eval('y')];
+			`,
+			out: '[11,22]'
+		});
+
+		describe('with prefix num change in 1st eval', () => {
+			itSerializes('with direct eval containing prefix num change', {
+				in: `
+					'use strict';
+					const x = 11, y = 22;
+					module.exports = [
+						eval('let livepack_temp; () => x'),
+						eval('() => y')
+					];
+				`,
+				out: '(()=>{const a=((a,b)=>[()=>a,()=>b])(11,22);return[a[0],a[1]]})()',
+				validate([fn1, fn2]) {
+					expect(fn1).toBeFunction();
+					expect(fn2).toBeFunction();
+					expect(fn1()).toBe(11);
+					expect(fn2()).toBe(22);
+				}
+			});
+
+			itSerializes('with indirect eval containing prefix num change', {
+				in: `
+					'use strict';
+					const y = 22;
+					module.exports = [
+						eval('let livepack_temp; () => 11'),
+						eval('() => y')
+					];
+				`,
+				out: '[()=>11,(a=>()=>a)(22)]',
+				validate([fn1, fn2]) {
+					expect(fn1).toBeFunction();
+					expect(fn2).toBeFunction();
+					expect(fn1()).toBe(11);
+					expect(fn2()).toBe(22);
+				}
+			});
+		});
+	});
 });