Use __defineSetter__ instead of toLocaleString

lib/init/with.js

@@ -3,6 +3,8 @@
  * Functions to handle `with` statements
  * ------------------*/
 
+/* eslint-disable no-restricted-properties, no-extend-native */
+
 'use strict';
 
 // Modules
@@ -27,7 +29,7 @@ let withState;
  *
  * 1. Tracker calls need to get the values of vars outside the `with ()`,
  *    but `with ()` could block access to them.
- * 2. `with ()` can block access to Livepack's internal vars (e.g. `livepack_tracker`)
+ * 2. `with ()` can block access to Livepack's internal vars (e.g. `livepack_tracker`),
  *    causing Livepack's internal mechanisms to malfunction.
  *
  * In both cases, it's necessary to "smuggle" the values of variables outside `with ()` to inside,
@@ -37,8 +39,8 @@ let withState;
  * and then retrieve them inside `with ()`.
  *
  * To avoid this being visible to user code, repurpose an existing global function
- * `Object.prototype.toLocaleString`. `toLocaleString` is chosen because it's fairly obscure,
- * but could use any method.
+ * `Object.prototype.__defineSetter__`.
+ * `__defineSetter__` is chosen because it's fairly obscure, but could use any method.
  *
  * `with (obj) x();` is instrumented as:
  *
@@ -49,11 +51,11 @@ let withState;
  *     (eval, livepack_temp_3) => eval(livepack_temp_3),
  *     () => eval
  *   )
- * ) with ( {}.toLocaleString() ) x();
+ * ) with ( {}.__defineSetter__() ) x();
  * ```
  *
- * `wrapWith()` stores the 2 functions in `withState`, and `{}.toLocaleString()` retrieves them again.
- * `toLocaleString()` returns a Proxy which allows dynamically overriding the original `with ()`
+ * `wrapWith()` stores the 2 functions in `withState`, and `{}.__defineSetter__()` retrieves them again.
+ * `__defineSetter__()` returns a Proxy which allows dynamically overriding the original `with ()`
  * by using the 2 functions `wrapWith()` was called with to access any var outside the `with ()`.
  * This Proxy is used as the value for a 2nd `with ()` which is inserted inside the original.
  *
@@ -66,34 +68,35 @@ function addWrapWithFunctionToTracker(tracker, prefixNum) {
 		// Don't do anything if `null` or `undefined`, as it will error
 		if (withValue == null) return withValue;
 
-		// Throw an error if user has changed value of `Object.prototype.toLocaleString`
-		const descriptor = Object.getOwnPropertyDescriptor(Object.prototype, 'toLocaleString');
+		// Throw an error if user has changed value of `Object.prototype.__defineSetter__`
+		const descriptor = Object.getOwnPropertyDescriptor(Object.prototype, '__defineSetter__');
 		assert(
 			// eslint-disable-next-line no-use-before-define
-			descriptor?.value === shimmedToLocaleString,
-			'Livepack shims `Object.prototype.toLocaleString` to instrument `with` statements.'
-			+ "It has been altered in use code, which prevents Livepack's correct functioning."
+			descriptor?.value === shimmedDefineSetter,
+			'Livepack shims `Object.prototype.__defineSetter__` to instrument `with` statements.'
+			+ "It has been altered in user code, which prevents Livepack's correct functioning."
 		);
 
-		// Store state for shimmed `toLocaleString` to retrieve
+		// Store state for shimmed `__defineSetter__` to retrieve
 		withState = [prefixNum, runEval, getEval];
 
 		// Return original `with` value
 		return withValue;
 	};
 }
 
-// Shim `Object.prototype.toLocaleString`.
+// Shim `Object.prototype.__defineSetter__`.
 // NB: This code runs before globals are catalogued.
 // Define replacement as a method, as original does not have a `prototype` property.
-const {toLocaleString} = Object.prototype;
-const shimmedToLocaleString = {
-	toLocaleString() {
+// 2 function params to maintain original's `.length` property.
+const defineSetter = Object.prototype.__defineSetter__;
+const shimmedDefineSetter = {
+	__defineSetter__(_x, _y) { // eslint-disable-line no-unused-vars
 		// If being used normally, defer to original
 		// eslint-disable-next-line prefer-rest-params
-		if (!withState) return toLocaleString.apply(this, arguments);
+		if (!withState) return defineSetter.apply(this, arguments);
 
-		// Is being used to augment `with`.
+		// Is being used to smuggle values into `with ()`.
 		// Get state previously stored by `wrapWith()`.
 		const [prefixNum, runEval, getEval] = withState;
 		withState = undefined;
@@ -106,7 +109,7 @@ const shimmedToLocaleString = {
 		// 2. Var being accessed is one of Livepack's internal vars.
 		// In these cases, intercept access which would otherwise hit the outer `with ()`,
 		// and instead use the `runEval()` function to get/set the variable outside `with ()`.
-		// If var being accessed is itself called `eval`, use `getEval()` instead.
+		// If var being accessed is called `eval`, use `getEval()` instead.
 		return new Proxy(Object.create(null), {
 			has(target, key) {
 				return getIsGettingScope() || key.startsWith(internalVarsPrefix);
@@ -115,12 +118,13 @@ const shimmedToLocaleString = {
 				return key === 'eval' ? getEval() : runEval(nativeEval, key);
 			},
 			set(target, key, value) {
-				const paramName = key === 'a' ? 'b' : 'a';
-				const set = runEval(nativeEval, `${paramName} => ${key} = ${paramName}`);
+				// Only used for setting internal temp vars, so no need to handle if key is `v` or `eval`
+				const set = runEval(nativeEval, `v => ${key} = v`);
 				set(value);
 				return true;
 			}
 		});
 	}
-}.toLocaleString;
-Object.prototype.toLocaleString = shimmedToLocaleString; // eslint-disable-line no-extend-native
+}.__defineSetter__;
+
+Object.prototype.__defineSetter__ = shimmedDefineSetter;

lib/instrument/visitors/with.js

@@ -74,7 +74,7 @@ function instrumentWithObj(node, binding, state) {
 	//     (eval, livepack_temp_3) => eval(livepack_temp_3),
 	//     () => eval
 	//   )
-	// ) with ( {}.toLocaleString() ) foo;
+	// ) with ( {}.__defineSetter__() ) foo;
 	// ```
 	const tempVarNode = createTempVarNode(state),
 		evalNode = t.identifier('eval');
@@ -96,9 +96,9 @@ function instrumentWithObj(node, binding, state) {
 		]
 	);
 
-	// `foo;` -> `with ( {}.toLocaleString() ) foo;`
+	// `foo;` -> `with ( {}.__defineSetter__() ) foo;`
 	node.body = t.withStatement(
-		t.callExpression(t.memberExpression(t.objectExpression([]), t.identifier('toLocaleString')), []),
+		t.callExpression(t.memberExpression(t.objectExpression([]), t.identifier('__defineSetter__')), []),
 		node.body
 	);
 }

test/with.test.js

@@ -564,13 +564,27 @@ describe('with statements', () => {
 		});
 	});
 
-	itSerializesEqual('Shimming `Object.prototype.toLocaleString` does not prevent serializing it', {
-		in: () => Object.prototype.toLocaleString,
-		out: 'Object.prototype.toLocaleString',
-		validate(toLocaleString) {
-			expect(toLocaleString).toBe(Object.prototype.toLocaleString);
-		}
+	/* eslint-disable no-restricted-properties */
+	describe('shimming `Object.prototype.__defineSetter__`', () => {
+		it('does not interfere with its normal functioning', () => {
+			const callee = spy();
+			const obj = {};
+			obj.__defineSetter__('foo', callee);
+			expect(callee).not.toHaveBeenCalled();
+			obj.foo = 123;
+			expect(callee).toHaveBeenCalledOnce();
+			expect(callee).toHaveBeenCalledWith(123);
+		});
+
+		itSerializesEqual('does not prevent serializing it', {
+			in: () => Object.prototype.__defineSetter__,
+			out: 'Object.prototype.__defineSetter__',
+			validate(defineSetter) {
+				expect(defineSetter).toBe(Object.prototype.__defineSetter__);
+			}
+		});
 	});
+	/* eslint-enable no-restricted-properties */
 });
 
 // TODO: Test for access to `eval` - as a global, and as local var.