fixup! review suggestions

Author: erights

packages/no-trapping-shim/shim.js

@@ -3,10 +3,8 @@ import { ReflectPlus, ObjectPlus, ProxyPlus } from './src/no-trapping-pony.js';
 
 globalThis.Reflect = ReflectPlus;
 
-// @ts-expect-error Something about the type of the Object constructor
 globalThis.Object = ObjectPlus;
 // eslint-disable-next-line no-extend-native
 Object.prototype.constructor = ObjectPlus;
 
-// @ts-expect-error Something about the type of Proxy
 globalThis.Proxy = ProxyPlus;

packages/no-trapping-shim/src/no-trapping-pony.js

@@ -1,7 +1,7 @@
 const OriginalObject = Object;
 const OriginalReflect = Reflect;
 const OriginalProxy = Proxy;
-const { freeze, defineProperty } = OriginalObject;
+const { freeze, defineProperty, hasOwn } = OriginalObject;
 const { apply, construct, ownKeys } = OriginalReflect;
 
 const noTrappingSet = new WeakSet();
@@ -144,55 +144,115 @@ const addExtras = (base, ...extrasArgs) => {
   }
 };
 
+/** In the shim, `ReflectPlus` replaces the global `Reflect`. */
 const ReflectPlus = {};
 addExtras(ReflectPlus, OriginalReflect, extraReflectMethods);
 export { ReflectPlus };
 
+/**
+ * In the shim, `ObjectPlus` replaces the global `Object`.
+ *
+ * @type {ObjectConstructor}
+ */
+// @ts-expect-error TS does not know the rest of the type is added below
 const ObjectPlus = function Object(...args) {
   if (new.target) {
     return construct(OriginalObject, args, new.target);
   } else {
     return apply(OriginalObject, this, args);
   }
 };
+// @ts-expect-error We actually can assign to its `.prototype`.
 ObjectPlus.prototype = OriginalObject.prototype;
 addExtras(ObjectPlus, OriginalObject, extraObjectMethods);
 export { ObjectPlus };
 
-const makeMetaHandler = handler =>
-  freeze({
-    get(_, trapName, _receiver) {
-      return freeze((target, ...rest) => {
-        if (
-          isNoTrappingInternal(target, true) ||
-          handler[trapName] === undefined
-        ) {
-          return ReflectPlus[trapName](target, ...rest);
-        } else {
-          return handler[trapName](target, ...rest);
+const metaHandler = freeze({
+  get(_, trapName, handlerPlus) {
+    /**
+     * The `trapPlus` method is an enhanced version of
+     * `originalHandler[trapName]`. If the handlerPlus has no own `trapName`
+     * property, then the `get` of the metaHandler is called, which returns
+     * the `trapPlus`, which is then called as the trap of the returned
+     * proxyPlus. When so called, it installs an own `handlerPlus[trapName]`
+     * which is either `undefined` or this same `trapPlus`, to avoid further
+     * need to meta-handle that `handlerPlus[trapName]`.
+     *
+     * @param {any} target
+     * @param {any[]} rest
+     */
+    const trapPlus = freeze((target, ...rest) => {
+      if (isNoTrappingInternal(target, true)) {
+        defineProperty(handlerPlus, trapName, {
+          value: undefined,
+          writable: false,
+          enumerable: true,
+          configurable: false,
+        });
+      } else {
+        if (!hasOwn(handlerPlus, trapName)) {
+          defineProperty(handlerPlus, trapName, {
+            value: trapPlus,
+            writable: false,
+            enumerable: true,
+            configurable: true,
+          });
         }
-      });
-    },
-  });
+        const { originalHandler } = handlerPlus;
+        const trap = originalHandler[trapName];
+        if (trap !== undefined) {
+          // Note that whether `trap === undefined` can change dynamically,
+          // so we do not install an own `handlerPlus[trapName] === undefined`
+          // for that case. We still install or preserve an own
+          // `handlerPlus[trapName] === trapPlus` until the target is
+          // seen to be non-trapping.
+          return apply(trap, originalHandler, [target, ...rest]);
+        }
+      }
+      return ReflectPlus[trapName](target, ...rest);
+    });
+    return trapPlus;
+  },
+});
 
-const makeSafeHandler = handler =>
-  new OriginalProxy({}, makeMetaHandler(handler));
+/**
+ * A handlerPlus starts as a fresh empty object that inherits from a proxy
+ * whose handler is the shared generic metaHandler.
+ * Thus, the metaHandler's `get` method is called only when the
+ * `handlerPlus` does not have a property overriding that `trapName`.
+ * In that case, the metaHandler's `get` is called with its `receiver`
+ * being the `handlerPlus`.
+ *
+ * @param {ProxyHandler<any>} originalHandler
+ * @returns {ProxyHandler<any> & {
+ *   isNoTrapping: (target: any) => boolean,
+ *   suppressTrapping: (target: any) => boolean,
+ *   originalHandler: ProxyHandler<any>
+ * }}
+ */
+const makeHandlerPlus = originalHandler => ({
+  // @ts-expect-error TS does not know what this __proto__ is doing
+  __proto__: new OriginalProxy({}, metaHandler),
+  // relies on there never being a trap named `originalHandler`.
+  originalHandler,
+});
 
 /**
- * In the shim, `ProxyPlus` should replace the global `Proxy`.
+ * In the shim, `ProxyPlus` replaces the global `Proxy`.
  *
- * @param {any} target
- * @param {object} handler
+ * @type {ProxyConstructor}
  */
+// @ts-expect-error We reject non-new calls in the body
 const ProxyPlus = function Proxy(target, handler) {
+  // @ts-expect-error Yes, we mean to compare these.
   if (new.target !== ProxyPlus) {
     if (new.target === undefined) {
       throw TypeError('Proxy constructor requires "new"');
     }
     throw TypeError('Safe Proxy shim does not support subclassing');
   }
-  const safeHandler = makeSafeHandler(handler);
-  const proxy = new OriginalProxy(target, safeHandler);
+  const handlerPlus = makeHandlerPlus(handler);
+  const proxy = new OriginalProxy(target, handlerPlus);
   proxyHandlerMap.set(proxy, [target, handler]);
   return proxy;
 };
@@ -201,10 +261,18 @@ const ProxyPlus = function Proxy(target, handler) {
 // `ProxyPlus.prototype` to `undefined`
 ProxyPlus.prototype = undefined;
 ProxyPlus.revocable = (target, handler) => {
-  const safeHandler = makeSafeHandler(handler);
-  const { proxy, revoke } = OriginalProxy.revocable(target, safeHandler);
+  const handlerPlus = makeHandlerPlus(handler);
+  const { proxy, revoke } = OriginalProxy.revocable(target, handlerPlus);
   proxyHandlerMap.set(proxy, [target, handler]);
-  return { proxy, revoke };
+  return {
+    proxy,
+    revoke() {
+      if (isNoTrappingInternal(target, true)) {
+        throw TypeError('Cannot revoke non-trapping proxy');
+      }
+      revoke();
+    },
+  };
 };
 
 export { ProxyPlus };