Ensure promise-typed operations and attributes return promises

Closes #79. This removes support for overloading promise operations and non-promise operations, since that would require a more ambitious approach and doesn't seem to appear on the web.
jsdom · Apr 26, 2020 · ecd4db4 · ecd4db4
1 parent a272dce
commit ecd4db4
Show file tree

Hide file tree

Showing 6 changed files with 272 additions and 1 deletion.
diff --git a/lib/constructs/attribute.js b/lib/constructs/attribute.js
@@ -23,6 +23,10 @@ class Attribute {
 
     const onInstance = utils.isOnInstance(this.idl, this.interface.idl);
 
+    const async = this.idl.idlType.generic === "Promise";
+    const promiseHandlingBefore = async ? `try {` : ``;
+    const promiseHandlingAfter = async ? `} catch (e) { return Promise.reject(e); }` : ``;
+
     let brandCheck = `
       if (!exports.is(esValue)) {
         throw new TypeError("Illegal invocation");
@@ -64,12 +68,18 @@ class Attribute {
     }
 
     addMethod(this.idl.name, [], `
+      ${promiseHandlingBefore}
       const esValue = this !== null && this !== undefined ? this : globalObject;
       ${brandCheck}
       ${getterBody}
+      ${promiseHandlingAfter}
     `, "get", { configurable });
 
     if (!this.idl.readonly) {
+      if (async) {
+        throw new Error(`Illegal promise-typed attribute "${this.idl.name}" in interface "${this.interface.idl.name}"`);
+      }
+
       let idlConversion;
       if (typeof this.idl.idlType.idlType === "string" && !this.idl.idlType.nullable &&
           this.ctx.enumerations.has(this.idl.idlType.idlType)) {

diff --git a/lib/constructs/operation.js b/lib/constructs/operation.js
@@ -27,6 +27,22 @@ class Operation {
     return firstOverloadOnInstance;
   }
 
+  isAsync() {
+    // As of the time of this writing, the current spec does not disallow such overloads, but the intention is to do so:
+    // https://github.com/heycam/webidl/pull/776
+    const firstAsync = this.idls[0].idlType.generic === "Promise";
+    for (const overload of this.idls.slice(1)) {
+      const isAsync = overload.idlType.generic === "Promise";
+      if (isAsync !== firstAsync) {
+        throw new Error(
+          `Overloading between Promise and non-Promise return types is not allowed: operation ` +
+          `"${this.name}" on ${this.interface.name}`
+        );
+      }
+    }
+    return firstAsync;
+  }
+
   fixUpArgsExtAttrs() {
     for (const idl of this.idls) {
       for (const arg of idl.arguments) {
@@ -50,6 +66,9 @@ class Operation {
     }
 
     const onInstance = this.isOnInstance();
+    const async = this.isAsync();
+    const promiseHandlingBefore = async ? `try {` : ``;
+    const promiseHandlingAfter = async ? `} catch (e) { return Promise.reject(e); }` : ``;
 
     const type = this.static ? "static operation" : "regular operation";
     const overloads = Overloads.getEffectiveOverloads(type, this.name, 0, this.interface);
@@ -100,6 +119,8 @@ class Operation {
     }
     str += invocation;
 
+    str = promiseHandlingBefore + str + promiseHandlingAfter;
+
     if (this.static) {
       this.interface.addStaticMethod(this.name, argNames, str);
     } else {

diff --git a/test/__snapshots__/test.js.snap b/test/__snapshots__/test.js.snap
@@ -446,6 +446,24 @@ exports.install = (globalObject, globalNames) => {
       }
     }
 
+    promiseOperation() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        CEReactions.preSteps(globalObject);
+        try {
+          return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation());
+        } finally {
+          CEReactions.postSteps(globalObject);
+        }
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
     get attr() {
       const esValue = this !== null && this !== undefined ? this : globalObject;
 
@@ -479,10 +497,31 @@ exports.install = (globalObject, globalNames) => {
         CEReactions.postSteps(globalObject);
       }
     }
+
+    get promiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        CEReactions.preSteps(globalObject);
+        try {
+          return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]);
+        } finally {
+          CEReactions.postSteps(globalObject);
+        }
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
   }
   Object.defineProperties(CEReactions.prototype, {
     method: { enumerable: true },
+    promiseOperation: { enumerable: true },
     attr: { enumerable: true },
+    promiseAttribute: { enumerable: true },
     [Symbol.toStringTag]: { value: \\"CEReactions\\", configurable: true }
   });
   if (globalObject[ctorRegistrySymbol] === undefined) {
@@ -3454,12 +3493,92 @@ exports.install = (globalObject, globalNames) => {
       }
       return esValue[implSymbol].promiseConsumer(...args);
     }
+
+    promiseOperation() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    unforgeablePromiseOperation() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol].unforgeablePromiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    get promiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]);
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    get unforgeablePromiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol][\\"unforgeablePromiseAttribute\\"]);
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    static staticPromiseOperation() {
+      try {
+        return utils.tryWrapperForImpl(Impl.implementation.staticPromiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    static get staticPromiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        return Impl.implementation[\\"staticPromiseAttribute\\"];
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
   }
   Object.defineProperties(PromiseTypes.prototype, {
     voidPromiseConsumer: { enumerable: true },
     promiseConsumer: { enumerable: true },
+    promiseOperation: { enumerable: true },
+    unforgeablePromiseOperation: { enumerable: true },
+    promiseAttribute: { enumerable: true },
+    unforgeablePromiseAttribute: { enumerable: true },
     [Symbol.toStringTag]: { value: \\"PromiseTypes\\", configurable: true }
   });
+  Object.defineProperties(PromiseTypes, {
+    staticPromiseOperation: { enumerable: true },
+    staticPromiseAttribute: { enumerable: true }
+  });
   if (globalObject[ctorRegistrySymbol] === undefined) {
     globalObject[ctorRegistrySymbol] = Object.create(null);
   }
@@ -8828,6 +8947,19 @@ exports.install = (globalObject, globalNames) => {
       return esValue[implSymbol].method();
     }
 
+    promiseOperation() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
     get attr() {
       const esValue = this !== null && this !== undefined ? this : globalObject;
 
@@ -8851,10 +8983,26 @@ exports.install = (globalObject, globalNames) => {
 
       esValue[implSymbol][\\"attr\\"] = V;
     }
+
+    get promiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]);
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
   }
   Object.defineProperties(CEReactions.prototype, {
     method: { enumerable: true },
+    promiseOperation: { enumerable: true },
     attr: { enumerable: true },
+    promiseAttribute: { enumerable: true },
     [Symbol.toStringTag]: { value: \\"CEReactions\\", configurable: true }
   });
   if (globalObject[ctorRegistrySymbol] === undefined) {
@@ -11810,12 +11958,92 @@ exports.install = (globalObject, globalNames) => {
       }
       return esValue[implSymbol].promiseConsumer(...args);
     }
+
+    promiseOperation() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol].promiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    unforgeablePromiseOperation() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol].unforgeablePromiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    get promiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol][\\"promiseAttribute\\"]);
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    get unforgeablePromiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        if (!exports.is(esValue)) {
+          throw new TypeError(\\"Illegal invocation\\");
+        }
+
+        return utils.tryWrapperForImpl(esValue[implSymbol][\\"unforgeablePromiseAttribute\\"]);
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    static staticPromiseOperation() {
+      try {
+        return utils.tryWrapperForImpl(Impl.implementation.staticPromiseOperation());
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
+
+    static get staticPromiseAttribute() {
+      try {
+        const esValue = this !== null && this !== undefined ? this : globalObject;
+
+        return Impl.implementation[\\"staticPromiseAttribute\\"];
+      } catch (e) {
+        return Promise.reject(e);
+      }
+    }
   }
   Object.defineProperties(PromiseTypes.prototype, {
     voidPromiseConsumer: { enumerable: true },
     promiseConsumer: { enumerable: true },
+    promiseOperation: { enumerable: true },
+    unforgeablePromiseOperation: { enumerable: true },
+    promiseAttribute: { enumerable: true },
+    unforgeablePromiseAttribute: { enumerable: true },
     [Symbol.toStringTag]: { value: \\"PromiseTypes\\", configurable: true }
   });
+  Object.defineProperties(PromiseTypes, {
+    staticPromiseOperation: { enumerable: true },
+    staticPromiseAttribute: { enumerable: true }
+  });
   if (globalObject[ctorRegistrySymbol] === undefined) {
     globalObject[ctorRegistrySymbol] = Object.create(null);
   }

diff --git a/test/cases/CEReactions.webidl b/test/cases/CEReactions.webidl
@@ -6,4 +6,7 @@ interface CEReactions {
     getter DOMString (DOMString name);
     [CEReactions] setter void (DOMString name, DOMString value);
     [CEReactions] deleter void (DOMString name);
+
+    [CEReactions] Promise<void> promiseOperation();
+    [CEReactions] readonly attribute Promise<void> promiseAttribute;
 };
diff --git a/test/cases/Overloads.webidl b/test/cases/Overloads.webidl
@@ -6,7 +6,7 @@ interface Overloads {
 
   DOMString compatible(DOMString arg1);
   byte compatible(DOMString arg1, DOMString arg2);
-  Promise<DOMString> compatible(DOMString arg1, DOMString arg2, optional long arg3 = 0);
+  URL compatible(DOMString arg1, DOMString arg2, optional long arg3 = 0);
 
   DOMString incompatible1(DOMString arg1);
   byte incompatible1(long arg1);

diff --git a/test/cases/PromiseTypes.webidl b/test/cases/PromiseTypes.webidl
@@ -2,4 +2,13 @@
 interface PromiseTypes {
   void voidPromiseConsumer(Promise<void> p);
   void promiseConsumer(Promise<double> p);
+
+  Promise<void> promiseOperation();
+  readonly attribute Promise<void> promiseAttribute;
+
+  [Unforgeable] Promise<void> unforgeablePromiseOperation();
+  [Unforgeable] readonly attribute Promise<void> unforgeablePromiseAttribute;
+
+  static Promise<void> staticPromiseOperation();
+  static readonly attribute Promise<void> staticPromiseAttribute;
 };