Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure promise-typed operations and attributes return promises #200

Merged
merged 2 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions lib/constructs/attribute.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -72,12 +76,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)) {
Expand Down
21 changes: 21 additions & 0 deletions lib/constructs/operation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linking to whatwg/webidl#776 would be useful.

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) {
Expand All @@ -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);
Expand Down Expand Up @@ -100,6 +119,8 @@ class Operation {
}
str += invocation;

str = promiseHandlingBefore + str + promiseHandlingAfter;

if (this.static) {
this.interface.addStaticMethod(this.name, argNames, str);
} else {
Expand Down
228 changes: 228 additions & 0 deletions test/__snapshots__/test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -3655,12 +3694,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);
}
Expand Down Expand Up @@ -9166,6 +9285,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;

Expand All @@ -9189,10 +9321,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) {
Expand Down Expand Up @@ -12349,12 +12497,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);
}
Expand Down
3 changes: 3 additions & 0 deletions test/cases/CEReactions.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
2 changes: 1 addition & 1 deletion test/cases/Overloads.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
9 changes: 9 additions & 0 deletions test/cases/PromiseTypes.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[Unforgeable] Promise<void> unforgeablePromiseOperation();
[Unforgeable] readonly attribute Promise<void> unforgeablePromiseAttribute;
[LegacyUnforgeable] Promise<void> unforgeablePromiseOperation();
[LegacyUnforgeable] readonly attribute Promise<void> unforgeablePromiseAttribute;


static Promise<void> staticPromiseOperation();
static readonly attribute Promise<void> staticPromiseAttribute;
};