From b78652115b62593fb91463e1e2448f1dc4806949 Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Wed, 17 Jan 2024 16:44:18 -0800 Subject: [PATCH] refactor(ses): Replace Compartment toString property with @@toStringTag Replaces the `Compartment` `toString` function with a `@@toStringTag` string property. This necessitated updating the permit for `%CompartmentPrototype%`, and some related tests. Some adjacent cleanup is also performed. --- packages/ses/src/compartment.js | 17 ++++++++--- packages/ses/src/permits.js | 3 +- packages/ses/test/error/throws-and-logs.js | 25 +++++----------- .../ses/test/test-compartment-instance.js | 19 ++---------- .../ses/test/test-compartment-prototype.js | 29 ++++++++++--------- 5 files changed, 40 insertions(+), 53 deletions(-) diff --git a/packages/ses/src/compartment.js b/packages/ses/src/compartment.js index 9220c4e57d..42232e9369 100644 --- a/packages/ses/src/compartment.js +++ b/packages/ses/src/compartment.js @@ -11,6 +11,7 @@ import { defineProperties, entries, promiseThen, + toStringTagSymbol, weakmapGet, weakmapSet, } from './commons.js'; @@ -105,10 +106,6 @@ export const CompartmentPrototype = { return compartmentEvaluate(compartmentFields, source, options); }, - toString() { - return '[object Compartment]'; - }, - module(specifier) { if (typeof specifier !== 'string') { throw TypeError('first argument of module() must be a string'); @@ -168,6 +165,18 @@ export const CompartmentPrototype = { }, }; +// This causes `String(new Compartment())` to evaluate to `[object Compartment]`. +// The descriptor follows the conventions of other globals with @@toStringTag +// properties, e.g. Math. +defineProperties(CompartmentPrototype, { + [toStringTagSymbol]: { + value: 'Compartment', + writable: false, + enumerable: false, + configurable: true, + }, +}); + defineProperties(InertCompartment, { prototype: { value: CompartmentPrototype }, }); diff --git a/packages/ses/src/permits.js b/packages/ses/src/permits.js index 569cabac50..ed9abc63c4 100644 --- a/packages/ses/src/permits.js +++ b/packages/ses/src/permits.js @@ -1516,12 +1516,11 @@ export const permitted = { evaluate: fn, globalThis: getter, name: getter, - // Should this be proposed? - toString: fn, import: asyncFn, load: asyncFn, importNow: fn, module: fn, + '@@toStringTag': 'string', }, lockdown: fn, diff --git a/packages/ses/test/error/throws-and-logs.js b/packages/ses/test/error/throws-and-logs.js index b1005b4727..f6ebd6d791 100644 --- a/packages/ses/test/error/throws-and-logs.js +++ b/packages/ses/test/error/throws-and-logs.js @@ -1,14 +1,12 @@ /* global globalThis */ -import { freeze, getPrototypeOf, is } from '../../src/commons.js'; -import { loggedErrorHandler, assert } from '../../src/error/assert.js'; +import { freeze, getPrototypeOf } from '../../src/commons.js'; +import { loggedErrorHandler } from '../../src/error/assert.js'; import { makeLoggingConsoleKit, makeCausalConsole, } from '../../src/error/console.js'; -const { quote: q } = assert; - // For our internal debugging purposes // const internalDebugConsole = console; @@ -19,11 +17,6 @@ const compareLogs = freeze((t, log, goldenLog) => { t.is(log.length, goldenLog.length, 'wrong log length'); log.forEach((logRecord, i) => { const goldenRecord = goldenLog[i]; - t.is( - logRecord.length, - goldenRecord.length, - `wrong length of log record ${i}`, - ); logRecord.forEach((logEntry, j) => { const goldenEntry = goldenRecord[j]; if ( @@ -33,16 +26,14 @@ const compareLogs = freeze((t, log, goldenLog) => { ) { t.assert(logEntry instanceof goldenEntry, 'not the right error'); } else { - // tap uses `===` instead of `Object.is`. - // Assuming ava does the right thing, switch back to this when - // switching back to ava. - // t.is(logEntry, goldenEntry); - t.assert( - is(logEntry, goldenEntry), - `${q(logEntry)} not same as ${q(goldenEntry)}`, - ); + t.is(logEntry, goldenEntry); } }); + t.is( + logRecord.length, + goldenRecord.length, + `wrong length of log record ${i}`, + ); }); }); diff --git a/packages/ses/test/test-compartment-instance.js b/packages/ses/test/test-compartment-instance.js index f5b465fd13..8b1b8f2add 100644 --- a/packages/ses/test/test-compartment-instance.js +++ b/packages/ses/test/test-compartment-instance.js @@ -2,7 +2,7 @@ import test from 'ava'; import '../index.js'; test('Compartment instance', t => { - t.plan(12); + t.plan(11); const c = new Compartment(); @@ -28,22 +28,7 @@ test('Compartment instance', t => { ); t.is(c.toString(), '[object Compartment]', 'toString()'); - t.is(c[Symbol.toStringTag], undefined, '"Symbol.toStringTag" property'); + t.is(c[Symbol.toStringTag], 'Compartment', '"Symbol.toStringTag" property'); t.deepEqual(Reflect.ownKeys(c), [], 'static properties'); - t.deepEqual( - Reflect.ownKeys(Object.getPrototypeOf(c)).sort(), - [ - 'constructor', - 'evaluate', - 'globalThis', - 'import', - 'importNow', - 'load', - 'module', - 'name', - 'toString', - ].sort(), - 'prototype properties', - ); }); diff --git a/packages/ses/test/test-compartment-prototype.js b/packages/ses/test/test-compartment-prototype.js index 7dae54ce0c..be92c5f261 100644 --- a/packages/ses/test/test-compartment-prototype.js +++ b/packages/ses/test/test-compartment-prototype.js @@ -12,19 +12,22 @@ test('Compartment prototype', t => { 'The initial value of Compartment.prototype.constructor', ); - t.deepEqual( - Reflect.ownKeys(Compartment.prototype).sort(), - [ - 'constructor', - 'evaluate', - 'globalThis', - 'import', - 'importNow', - 'load', - 'module', - 'name', - 'toString', - ].sort(), + const expectedProps = new Set([ + 'constructor', + 'evaluate', + 'globalThis', + 'import', + 'importNow', + 'load', + 'module', + 'name', + Symbol.toStringTag, + ]); + const actualProps = Reflect.ownKeys(Compartment.prototype); + + t.assert( + actualProps.length === expectedProps.size && + actualProps.every(key => expectedProps.has(key)), 'prototype properties', ); });