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.

Sign up for GitHub

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

Jump to bottom

Implement callback interfaces #178

Closed
wants to merge 8 commits into from
Closed

Implement callback interfaces #178

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9d673c4
feat(constructs): Add support for callback interfaces
ExE-Boss Mar 7, 2020
48d968f
fix(callback‑interface): Generate `requires` block
ExE-Boss Mar 8, 2020
2b1f360
fix(callback‑interface): Expose wrapped value
ExE-Boss Mar 8, 2020
2f01f5a
fix(callback‑interface): Address review comments
ExE-Boss Mar 8, 2020
9cd8797
fix(callback‑interface): Update README and remove non‑standard cache
ExE-Boss Mar 9, 2020
a8d0154
Nits, mostly formatting
domenic Mar 14, 2020
28543f0
Small tweaks to generated code
domenic Mar 14, 2020
38e0248
Avoid loop in generated code, loop in source instead
domenic Mar 14, 2020
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ If any part of the conversion fails, _context_ can be used to describe the provi

#### `install(globalObject)`

If this callback interface has constants, then this method creates a brand new legacy callback interface object and attaches it to the passed `globalObject`, otherwise, this method is a no-op.
If this callback interface has constants, then this method creates a brand new legacy callback interface object and attaches it to the passed `globalObject`. Otherwise, this method is a no-op.

### For dictionaries

Expand Down
25 changes: 6 additions & 19 deletions lib/constructs/callback-interface.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ class CallbackInterface {
}
}

addStaticProperty(name, body, {
configurable = true,
enumerable = typeof name === "string",
writable = true
} = {}) {
addStaticProperty(name, body, { configurable = true, enumerable = typeof name === "string", writable = true } = {}) {
const descriptor = { configurable, enumerable, writable };
this._outputStaticProperties.set(name, { body, descriptor });
}
Expand All @@ -69,12 +65,10 @@ class CallbackInterface {
generateConversion() {
const { operation, name } = this;
const opName = operation.name;
const isAsync =
(operation.idlType.union &&
operation.idlType.idlType.some(
idlType => idlType.generic === "Promise"
)) ||
operation.idlType.generic === "Promise";

const isAsyncInUnion = operation.idlType.union &&
operation.idlType.idlType.some(idlType => idlType.generic === "Promise");
const isAsync = isAsyncInUnion || operation.idlType.generic === "Promise";

const argsLength = operation.arguments.length;
this.str += `
Expand Down Expand Up @@ -125,14 +119,7 @@ class CallbackInterface {
`;

if (operation.idlType.idlType !== "void") {
const conv = Types.generateTypeConversion(
this.ctx,
"callResult",
operation.idlType,
[],
name,
"context"
);
const conv = Types.generateTypeConversion(this.ctx, "callResult", operation.idlType, [], name, "context");
this.requires.merge(conv.requires);
this.str += `
${conv.body}
Expand Down
9 changes: 1 addition & 8 deletions lib/transformer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,7 @@ class Transformer {
}));

this.ctx.initialize();
const {
interfaces,
interfaceMixins,
callbackInterfaces,
dictionaries,
enumerations,
typedefs
} = this.ctx;
const { interfaces, interfaceMixins, callbackInterfaces, dictionaries, enumerations, typedefs } = this.ctx;

// first we're gathering all full interfaces and ignore partial ones
for (const file of parsed) {
Expand Down
7 changes: 1 addition & 6 deletions lib/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,7 @@ function mergeExtAttrs(a = [], b = []) {
}

// Types of types that generate an output file.
const resolvedTypes = new Set([
"callback interface",
"dictionary",
"enumeration",
"interface"
]);
const resolvedTypes = new Set(["callback interface", "dictionary", "enumeration", "interface"]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I made this multi‑line so that when there’s multiple in‑flight PRs that add new types (like #123), they avoid conflicting with each other.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that's a sufficient motivation for making the code more verbose in that way. In general, please do not change project style without opening an issue for discussion.


function resolveType(ctx, idlType, stack = []) {
if (resolvedMap.has(idlType)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ function getPropertyDescriptorModifier(currentDesc, targetDesc, type, value = un
return `{ ${changes.join(", ")} }`;
}

const defaultDefinePropertyDescriptor = Object.freeze({
const defaultDefinePropertyDescriptor = {
configurable: false,
enumerable: false,
writable: false
});
};

class RequiresMap extends Map {
constructor(ctx) {
Expand Down