refactor(ses): Replace Compartment toString property with @@toStringT…

…ag (merge #1962)
endojs · Jan 19, 2024 · c295b02 · c295b02
2 parents 05d46d6 + b786521
commit c295b02
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 53 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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',
   );
 });