From e9cc91d9932294ca6399beda9ab5e0fca31683db Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Sun, 22 Mar 2020 15:36:36 -0400 Subject: [PATCH 1/5] Add proper reflection support through processor It has long been proposed that reflection features be moved to the host-side, considering it is a part of HTML rather than Web IDL. With the recently added processor infrastructure, this is now possible. With this change, non-relative imports are now allowed in processors' addImport function. This is done by checking if the imported package fits the npm package name regular expression. Thanks to ExE Boss for this contribution. We also note that existing reflectors did not deal very well with the change from this to esValue, since 2a2a0999a950106094e83ec9d12ea91bad51b0e2. Since this commit moves the reflection code out of webidl2js, the bug now naturally disappears. Closes #15. Closes #93. Co-authored-by: ExE Boss <3889017+ExE-Boss@users.noreply.github.com> --- README.md | 83 +++++++++++--- lib/constructs/attribute.js | 15 +-- lib/constructs/dictionary.js | 2 +- lib/constructs/interface.js | 2 +- lib/context.js | 14 ++- lib/parameters.js | 2 +- lib/reflector.js | 52 --------- lib/transformer.js | 1 + lib/types.js | 4 +- lib/utils.js | 27 ++++- test/__snapshots__/test.js.snap | 195 ++++++++++++++++++++------------ test/cases/Reflect.webidl | 11 +- test/reflector.js | 64 +++++++++++ test/test.js | 30 +++++ 14 files changed, 336 insertions(+), 166 deletions(-) delete mode 100644 lib/reflector.js create mode 100644 test/reflector.js diff --git a/README.md b/README.md index 07e2a78b..8249c021 100644 --- a/README.md +++ b/README.md @@ -128,15 +128,15 @@ By default webidl2js ignores HTML Standard-defined extended attributes [`[CEReac Both hooks have the signature `(code) => replacementCode`, where: -- `code` is the code generated by webidl2js normally, for calling into the impl class. +- `code` (string) is the code generated by webidl2js normally, for calling into the impl class. -- `replacementCode` is the new code that will be output in place of `code` in the wrapper class. +- `replacementCode` (string) is the new code that will be output in place of `code` in the wrapper class. If either hook is omitted, then the code will not be replaced, i.e. the default is equivalent to `(code) => code`. -Both hooks also have some utility methods that are accessible via `this`: +Both hooks also has a utility method that is accessible via `this`: -- `addImport(relPath, [importedIdentifier])` utility to require external modules from the generated interface. This method accepts 2 parameters: `relPath` the relative path from the generated interface file, and an optional `importedIdentifier` the identifier to import. This method returns the local identifier from the imported path. +- `addImport(path, [importedIdentifier])` utility to require external modules from the generated interface. This method accepts 2 parameters: `path` the relative or absolute path from the generated interface file, and an optional `importedIdentifier` the identifier to import. This method returns the local identifier from the imported path. The following variables are available in the scope of the replacement code: @@ -182,6 +182,69 @@ transformer.generate("wrappers").catch(err => { }); ``` +### `[Reflect]` + +Many HTML IDL attributes are defined using [reflecting a content attribute to an IDL attribute](https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflect). By default webidl2js doesn't do much with reflection, since it requires detailed knowledge of the host environment to implement correctly. However, we offer the `processReflect` processor hook to allow the host environment to automate the task of implementing reflected IDL attributes. + +The `processReflect` processor hook has the signature `(idl, implName) => ({ get, set })`, where: + +- `idl` is the [`attribute` AST node](https://github.com/w3c/webidl2.js/#attribute-member) as emitted by the [webidl2](https://github.com/w3c/webidl2.js) parser. + +- `implName` (string) is a JavaScript expression that would evaluate to the implementation object at runtime. + +- `get` (string) is the code that will be output in the attribute getter, after any function prologue. + +- `set` (string) is the code that will be output in the attribute setter, after any function prologue. + +The hook also has a utility method that is accessible via `this`: + +- `addImport(path, [importedIdentifier])` utility to require external modules from the generated interface. This method accepts 2 parameters: `path` the relative or absolute path from the generated interface file, and an optional `importedIdentifier` the identifier to import. This method returns the local identifier from the imported path. + +The following variables are available in the scope of the replacement code: + +- `globalObject` (object) is the global object associated with the interface + +- `interfaceName` (string) is the name of the interface + +- (for setter only) `V` (any) is the converted input to the setter method. + +To mark an attribute as reflected, an extended attribute whose name starts with `Reflect` should be added to the IDL attribute. This means that any of the following is treated as reflected by webidl2js: + +```webidl +[Reflect] attribute boolean reflectedBoolean; +[ReflectURL] attribute USVString reflectedURL; +[Reflect=value, ReflectURL] attribute USVString reflectedValue; +``` + +webidl2js itself does not particularly care about the particular reflection-related extended attribute(s) being used, only that one exists. However, your processor hook can make use of the extended attributes for additional information on how the attribute is reflected. + +An example processor function that implements `boolean` IDL attribute reflection is as follows: + +```js +function processReflect(idl, implName) { + // Determine the name of the content attribute being reflected as follows: + // - if [Reflect] is provided with a value, then the value is the content attribute. + // - otherwise, the content attribute is the name of the IDL attribute, lowercased. + const reflectAttr = idl.extAttrs.find(attr => attr.name === "Reflect"); + const attrName = + reflectAttr && reflectAttr.rhs && reflectAttr.rhs.value.replace(/_/g, "-") || idl.name.toLowerCase(); + + if (idl.idlType.idlType === "boolean") { + return { + get: `return ${implName}.hasAttributeNS(null, "${attrName}");`, + set: ` + if (V) { + ${implName}.setAttributeNS(null, "${attrName}", ""); + } else { + ${implName}.removeAttributeNS(null, "${attrName}"); + } + ` + }; + } + throw new Error(`Unknown IDL type: ${idl.idlType.idlType}`); +} +``` + ## Generated wrapper class file API The example above showed a simplified generated wrapper file with only three exports: `create`, `is`, and `interface`. In reality the generated wrapper file will contain more functionality, documented here. This functionality is different between generated wrapper files for interfaces and for dictionaries. @@ -414,17 +477,7 @@ Notable missing features include: ## Nonstandard extended attributes -A couple of non-standard extended attributes are baked in to webidl2js. - -### `[Reflect]` - -The `[Reflect]` extended attribute is used on IDL attributes to implement the rules for [reflecting a content attribute to an IDL attribute](https://html.spec.whatwg.org/multipage/common-dom-interfaces.html#reflect). If `[Reflect]` is specified, the implementation class does not need to implement any getter or setter logic; webidl2js will take care of it. - -By default the attribute passed to `this.getAttribute` and `this.setAttribute` will be the same as the name of the property being reflected. You can use the form `[Reflect=custom]` or `[Reflect=custom_with_dashes]` to change that to be `"custom"` or `"custom-with-dashes"`, respectively. - -Note that only the basics of the reflect algorithm are implemented so far: `boolean`, `DOMString`, `long`, and `unsigned long`, with no parametrizations. - -In the future we may change this extended attribute to be handled by the caller, similar to `[CEReactions]` and `[HTMLConstructor]`, since it is more related to HTML than to Web IDL. +A non-standard extended attribute is baked in to webidl2js. ### `[WebIDL2JSValueAsUnsupported=value]` diff --git a/lib/constructs/attribute.js b/lib/constructs/attribute.js index cd2e9d56..fdeb2b7a 100644 --- a/lib/constructs/attribute.js +++ b/lib/constructs/attribute.js @@ -3,7 +3,6 @@ const conversions = require("webidl-conversions"); const utils = require("../utils"); -const reflector = require("../reflector"); const Types = require("../types"); class Attribute { @@ -18,7 +17,8 @@ class Attribute { const requires = new utils.RequiresMap(this.ctx); const configurable = !utils.getExtAttr(this.idl.extAttrs, "Unforgeable"); - const shouldReflect = utils.getExtAttr(this.idl.extAttrs, "Reflect"); + const shouldReflect = + this.idl.extAttrs.some(attr => attr.name.startsWith("Reflect")) && this.ctx.processReflect !== null; const sameObject = utils.getExtAttr(this.idl.extAttrs, "SameObject"); const onInstance = utils.isOnInstance(this.idl, this.interface.idl); @@ -43,12 +43,9 @@ class Attribute { getterBody = `return Impl.implementation["${this.idl.name}"];`; setterBody = `Impl.implementation["${this.idl.name}"] = V;`; } else if (shouldReflect) { - if (!reflector[this.idl.idlType.idlType]) { - throw new Error("Unknown reflector type: " + this.idl.idlType.idlType); - } - const attrName = shouldReflect.rhs && shouldReflect.rhs.value.replace(/_/g, "-") || this.idl.name; - getterBody = reflector[this.idl.idlType.idlType].get("esValue", attrName.toLowerCase()); - setterBody = reflector[this.idl.idlType.idlType].set("esValue", attrName.toLowerCase()); + const processedOutput = this.ctx.invokeProcessReflect(this.idl, "esValue[impl]", { requires }); + getterBody = processedOutput.get; + setterBody = processedOutput.set; } if (utils.getExtAttr(this.idl.extAttrs, "LenientThis")) { @@ -76,7 +73,7 @@ class Attribute { let idlConversion; if (typeof this.idl.idlType.idlType === "string" && !this.idl.idlType.nullable && this.ctx.enumerations.has(this.idl.idlType.idlType)) { - requires.add(this.idl.idlType.idlType); + requires.addRelative(this.idl.idlType.idlType); idlConversion = ` V = \`\${V}\`; if (!${this.idl.idlType.idlType}.enumerationValues.has(V)) { diff --git a/lib/constructs/dictionary.js b/lib/constructs/dictionary.js index dcefdd58..97de5f74 100644 --- a/lib/constructs/dictionary.js +++ b/lib/constructs/dictionary.js @@ -95,7 +95,7 @@ class Dictionary { `; if (this.idl.inheritance) { - this.requires.add(this.idl.inheritance); + this.requires.addRelative(this.idl.inheritance); } this.str = ` ${this.requires.generate()} diff --git a/lib/constructs/interface.js b/lib/constructs/interface.js index 66abe37d..719808aa 100644 --- a/lib/constructs/interface.js +++ b/lib/constructs/interface.js @@ -496,7 +496,7 @@ class Interface { this.requires.addRaw("ctorRegistry", "utils.ctorRegistrySymbol"); if (this.idl.inheritance !== null) { - this.requires.add(this.idl.inheritance); + this.requires.addRelative(this.idl.inheritance); } this.str = ` diff --git a/lib/context.js b/lib/context.js index 48343ca0..05f7ae7f 100644 --- a/lib/context.js +++ b/lib/context.js @@ -19,11 +19,13 @@ class Context { implSuffix = "", processCEReactions = defaultProcessor, processHTMLConstructor = defaultProcessor, + processReflect = null, options } = {}) { this.implSuffix = implSuffix; this.processCEReactions = processCEReactions; this.processHTMLConstructor = processHTMLConstructor; + this.processReflect = processReflect; this.options = options; this.initialize(); @@ -58,14 +60,18 @@ class Context { } invokeProcessCEReactions(code, config) { - return this._invokeProcessor(this.processCEReactions, code, config); + return this._invokeProcessor(this.processCEReactions, config, code); } invokeProcessHTMLConstructor(code, config) { - return this._invokeProcessor(this.processHTMLConstructor, code, config); + return this._invokeProcessor(this.processHTMLConstructor, config, code); } - _invokeProcessor(processor, code, config) { + invokeProcessReflect(idl, implName, config) { + return this._invokeProcessor(this.processReflect, config, idl, implName); + } + + _invokeProcessor(processor, config, ...args) { const { requires } = config; if (!requires) { @@ -78,7 +84,7 @@ class Context { } }; - return processor.call(context, code); + return processor.apply(context, args); } } diff --git a/lib/parameters.js b/lib/parameters.js index 843f8e3e..b73aeab8 100644 --- a/lib/parameters.js +++ b/lib/parameters.js @@ -131,7 +131,7 @@ module.exports.generateOverloadConversions = function (ctx, typeOfOp, name, pare // Avoid requiring the interface itself if (iface !== parent.name) { fn = `${iface}.is`; - requires.add(iface); + requires.addRelative(iface); } else { fn = "exports.is"; } diff --git a/lib/reflector.js b/lib/reflector.js deleted file mode 100644 index 4c109488..00000000 --- a/lib/reflector.js +++ /dev/null @@ -1,52 +0,0 @@ -"use strict"; - -module.exports.boolean = { - get(objName, attrName) { - return `return this[impl].hasAttributeNS(null, "${attrName}");`; - }, - set(objName, attrName) { - return ` - if (V) { - this[impl].setAttributeNS(null, "${attrName}", ""); - } else { - this[impl].removeAttributeNS(null, "${attrName}"); - } - `; - } -}; - -module.exports.DOMString = { - get(objName, attrName) { - return ` - const value = this[impl].getAttributeNS(null, "${attrName}"); - return value === null ? "" : value; - `; - }, - set(objName, attrName) { - return `this[impl].setAttributeNS(null, "${attrName}", V);`; - } -}; - -module.exports.long = { - get(objName, attrName) { - return ` - const value = parseInt(this[impl].getAttributeNS(null, "${attrName}")); - return isNaN(value) || value < -2147483648 || value > 2147483647 ? 0 : value - `; - }, - set(objName, attrName) { - return `this[impl].setAttributeNS(null, "${attrName}", String(V));`; - } -}; - -module.exports["unsigned long"] = { - get(objName, attrName) { - return ` - const value = parseInt(this[impl].getAttributeNS(null, "${attrName}")); - return isNaN(value) || value < 0 || value > 2147483647 ? 0 : value - `; - }, - set(objName, attrName) { - return `this[impl].setAttributeNS(null, "${attrName}", String(V > 2147483647 ? 0 : V));`; - } -}; diff --git a/lib/transformer.js b/lib/transformer.js index b7297b57..ec480b31 100644 --- a/lib/transformer.js +++ b/lib/transformer.js @@ -19,6 +19,7 @@ class Transformer { implSuffix: opts.implSuffix, processCEReactions: opts.processCEReactions, processHTMLConstructor: opts.processHTMLConstructor, + processReflect: opts.processReflect, options: { suppressErrors: Boolean(opts.suppressErrors) } diff --git a/lib/types.js b/lib/types.js index 9f572155..596979ad 100644 --- a/lib/types.js +++ b/lib/types.js @@ -122,7 +122,7 @@ function generateTypeConversion(ctx, name, idlType, argAttrs = [], parentName, e // Avoid requiring the interface itself if (idlType.idlType !== parentName) { fn = `${idlType.idlType}.convert`; - requires.add(idlType.idlType); + requires.addRelative(idlType.idlType); } else { fn = `exports.convert`; } @@ -174,7 +174,7 @@ function generateTypeConversion(ctx, name, idlType, argAttrs = [], parentName, e // Avoid requiring the interface itself if (iface !== parentName) { fn = `${iface}.is`; - requires.add(iface); + requires.addRelative(iface); } else { fn = "exports.is"; } diff --git a/lib/utils.js b/lib/utils.js index 598d2f10..d83285bb 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -1,4 +1,5 @@ "use strict"; +const { extname } = require("path"); function getDefault(dflt) { switch (dflt.type) { @@ -65,14 +66,36 @@ function stringifyPropertyName(prop) { return typeof prop === "symbol" ? symbolName(prop) : JSON.stringify(propertyName(prop)); } +function toKey(type, func = "") { + return String(func + type).replace(/[./-]+/g, " ").trim().replace(/ /g, "_"); +} + +const PACKAGE_NAME_REGEX = /^(?:@([^/]+?)[/])?([^/]+?)$/u; + class RequiresMap extends Map { constructor(ctx) { super(); this.ctx = ctx; } - add(type, func = "") { - const key = (func + type).replace(/[./-]+/g, " ").trim().replace(/ /g, "_"); + add(name, func = "") { + const key = toKey(name, func); + + // If `name` is a package name or has a file extension, then use it as-is, + // otherwise append the `.js` file extension: + const importPath = PACKAGE_NAME_REGEX.test(name) || extname(name) ? name : `${name}.js`; + let req = `require(${JSON.stringify(importPath)})`; + + if (func) { + req += `.${func}`; + } + + this.addRaw(key, req); + return key; + } + + addRelative(type, func = "") { + const key = toKey(type, func); const path = type.startsWith(".") ? type : `./${type}`; let req = `require("${path}.js")`; diff --git a/test/__snapshots__/test.js.snap b/test/__snapshots__/test.js.snap index 973ce5ed..0eaf40cd 100644 --- a/test/__snapshots__/test.js.snap +++ b/test/__snapshots__/test.js.snap @@ -2194,6 +2194,7 @@ exports[`with processors Reflect.webidl 1`] = ` const conversions = require(\\"webidl-conversions\\"); const utils = require(\\"./utils.js\\"); +const whatwg_url = require(\\"whatwg-url\\"); const impl = utils.implSymbol; const ctorRegistry = utils.ctorRegistrySymbol; @@ -2253,17 +2254,17 @@ exports.install = function install(globalObject) { throw new TypeError(\\"Illegal constructor\\"); } - get ReflectedBoolean() { + get reflectedBoolean() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - return this[impl].hasAttributeNS(null, \\"reflectedboolean\\"); + return esValue[impl].hasAttributeNS(null, \\"reflectedboolean\\"); } - set ReflectedBoolean(V) { + set reflectedBoolean(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -2271,28 +2272,28 @@ exports.install = function install(globalObject) { } V = conversions[\\"boolean\\"](V, { - context: \\"Failed to set the 'ReflectedBoolean' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedBoolean' property on 'Reflect': The provided value\\" }); if (V) { - this[impl].setAttributeNS(null, \\"reflectedboolean\\", \\"\\"); + esValue[impl].setAttributeNS(null, \\"reflectedboolean\\", \\"\\"); } else { - this[impl].removeAttributeNS(null, \\"reflectedboolean\\"); + esValue[impl].removeAttributeNS(null, \\"reflectedboolean\\"); } } - get ReflectedDOMString() { + get reflectedDOMString() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = this[impl].getAttributeNS(null, \\"reflecteddomstring\\"); + const value = esValue[impl].getAttributeNS(null, \\"reflecteddomstring\\"); return value === null ? \\"\\" : value; } - set ReflectedDOMString(V) { + set reflectedDOMString(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -2300,24 +2301,24 @@ exports.install = function install(globalObject) { } V = conversions[\\"DOMString\\"](V, { - context: \\"Failed to set the 'ReflectedDOMString' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedDOMString' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflecteddomstring\\", V); + esValue[impl].setAttributeNS(null, \\"reflecteddomstring\\", V); } - get ReflectedLong() { + get reflectedLong() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = parseInt(this[impl].getAttributeNS(null, \\"reflectedlong\\")); + const value = parseInt(esValue[impl].getAttributeNS(null, \\"reflectedlong\\")); return isNaN(value) || value < -2147483648 || value > 2147483647 ? 0 : value; } - set ReflectedLong(V) { + set reflectedLong(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -2325,24 +2326,24 @@ exports.install = function install(globalObject) { } V = conversions[\\"long\\"](V, { - context: \\"Failed to set the 'ReflectedLong' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedLong' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflectedlong\\", String(V)); + esValue[impl].setAttributeNS(null, \\"reflectedlong\\", String(V)); } - get ReflectedUnsignedLong() { + get reflectedUnsignedLong() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = parseInt(this[impl].getAttributeNS(null, \\"reflectedunsignedlong\\")); + const value = parseInt(esValue[impl].getAttributeNS(null, \\"reflectedunsignedlong\\")); return isNaN(value) || value < 0 || value > 2147483647 ? 0 : value; } - set ReflectedUnsignedLong(V) { + set reflectedUnsignedLong(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -2350,24 +2351,53 @@ exports.install = function install(globalObject) { } V = conversions[\\"unsigned long\\"](V, { - context: \\"Failed to set the 'ReflectedUnsignedLong' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedUnsignedLong' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflectedunsignedlong\\", String(V > 2147483647 ? 0 : V)); + esValue[impl].setAttributeNS(null, \\"reflectedunsignedlong\\", String(V > 2147483647 ? 0 : V)); } - get ReflectionTest() { + get reflectedUSVStringURL() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = this[impl].getAttributeNS(null, \\"reflection\\"); + const value = esValue[impl].getAttributeNS(null, \\"reflectedusvstringurl\\"); + if (value === null) { + return \\"\\"; + } + const urlRecord = whatwg_url.parseURL(value, { baseURL: \\"http://localhost:8080/\\" }); + return urlRecord === null ? conversions.USVString(value) : whatwg_url.serializeURL(urlRecord); + } + + set reflectedUSVStringURL(V) { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + V = conversions[\\"USVString\\"](V, { + context: \\"Failed to set the 'reflectedUSVStringURL' property on 'Reflect': The provided value\\" + }); + + esValue[impl].setAttributeNS(null, \\"reflectedusvstringurl\\", V); + } + + get reflectionTest() { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + const value = esValue[impl].getAttributeNS(null, \\"reflection\\"); return value === null ? \\"\\" : value; } - set ReflectionTest(V) { + set reflectionTest(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -2375,10 +2405,10 @@ exports.install = function install(globalObject) { } V = conversions[\\"DOMString\\"](V, { - context: \\"Failed to set the 'ReflectionTest' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectionTest' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflection\\", V); + esValue[impl].setAttributeNS(null, \\"reflection\\", V); } get withUnderscore() { @@ -2388,7 +2418,7 @@ exports.install = function install(globalObject) { throw new TypeError(\\"Illegal invocation\\"); } - const value = this[impl].getAttributeNS(null, \\"with-underscore\\"); + const value = esValue[impl].getAttributeNS(null, \\"with-underscore\\"); return value === null ? \\"\\" : value; } @@ -2403,15 +2433,16 @@ exports.install = function install(globalObject) { context: \\"Failed to set the 'withUnderscore' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"with-underscore\\", V); + esValue[impl].setAttributeNS(null, \\"with-underscore\\", V); } } Object.defineProperties(Reflect.prototype, { - ReflectedBoolean: { enumerable: true }, - ReflectedDOMString: { enumerable: true }, - ReflectedLong: { enumerable: true }, - ReflectedUnsignedLong: { enumerable: true }, - ReflectionTest: { enumerable: true }, + reflectedBoolean: { enumerable: true }, + reflectedDOMString: { enumerable: true }, + reflectedLong: { enumerable: true }, + reflectedUnsignedLong: { enumerable: true }, + reflectedUSVStringURL: { enumerable: true }, + reflectionTest: { enumerable: true }, withUnderscore: { enumerable: true }, [Symbol.toStringTag]: { value: \\"Reflect\\", configurable: true } }); @@ -9247,17 +9278,17 @@ exports.install = function install(globalObject) { throw new TypeError(\\"Illegal constructor\\"); } - get ReflectedBoolean() { + get reflectedBoolean() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - return this[impl].hasAttributeNS(null, \\"reflectedboolean\\"); + return esValue[impl][\\"reflectedBoolean\\"]; } - set ReflectedBoolean(V) { + set reflectedBoolean(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -9265,28 +9296,23 @@ exports.install = function install(globalObject) { } V = conversions[\\"boolean\\"](V, { - context: \\"Failed to set the 'ReflectedBoolean' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedBoolean' property on 'Reflect': The provided value\\" }); - if (V) { - this[impl].setAttributeNS(null, \\"reflectedboolean\\", \\"\\"); - } else { - this[impl].removeAttributeNS(null, \\"reflectedboolean\\"); - } + esValue[impl][\\"reflectedBoolean\\"] = V; } - get ReflectedDOMString() { + get reflectedDOMString() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = this[impl].getAttributeNS(null, \\"reflecteddomstring\\"); - return value === null ? \\"\\" : value; + return esValue[impl][\\"reflectedDOMString\\"]; } - set ReflectedDOMString(V) { + set reflectedDOMString(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -9294,24 +9320,23 @@ exports.install = function install(globalObject) { } V = conversions[\\"DOMString\\"](V, { - context: \\"Failed to set the 'ReflectedDOMString' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedDOMString' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflecteddomstring\\", V); + esValue[impl][\\"reflectedDOMString\\"] = V; } - get ReflectedLong() { + get reflectedLong() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = parseInt(this[impl].getAttributeNS(null, \\"reflectedlong\\")); - return isNaN(value) || value < -2147483648 || value > 2147483647 ? 0 : value; + return esValue[impl][\\"reflectedLong\\"]; } - set ReflectedLong(V) { + set reflectedLong(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -9319,24 +9344,23 @@ exports.install = function install(globalObject) { } V = conversions[\\"long\\"](V, { - context: \\"Failed to set the 'ReflectedLong' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedLong' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflectedlong\\", String(V)); + esValue[impl][\\"reflectedLong\\"] = V; } - get ReflectedUnsignedLong() { + get reflectedUnsignedLong() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = parseInt(this[impl].getAttributeNS(null, \\"reflectedunsignedlong\\")); - return isNaN(value) || value < 0 || value > 2147483647 ? 0 : value; + return esValue[impl][\\"reflectedUnsignedLong\\"]; } - set ReflectedUnsignedLong(V) { + set reflectedUnsignedLong(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -9344,24 +9368,47 @@ exports.install = function install(globalObject) { } V = conversions[\\"unsigned long\\"](V, { - context: \\"Failed to set the 'ReflectedUnsignedLong' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectedUnsignedLong' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflectedunsignedlong\\", String(V > 2147483647 ? 0 : V)); + esValue[impl][\\"reflectedUnsignedLong\\"] = V; } - get ReflectionTest() { + get reflectedUSVStringURL() { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { throw new TypeError(\\"Illegal invocation\\"); } - const value = this[impl].getAttributeNS(null, \\"reflection\\"); - return value === null ? \\"\\" : value; + return esValue[impl][\\"reflectedUSVStringURL\\"]; } - set ReflectionTest(V) { + set reflectedUSVStringURL(V) { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + V = conversions[\\"USVString\\"](V, { + context: \\"Failed to set the 'reflectedUSVStringURL' property on 'Reflect': The provided value\\" + }); + + esValue[impl][\\"reflectedUSVStringURL\\"] = V; + } + + get reflectionTest() { + const esValue = this !== null && this !== undefined ? this : globalObject; + + if (!exports.is(esValue)) { + throw new TypeError(\\"Illegal invocation\\"); + } + + return esValue[impl][\\"reflectionTest\\"]; + } + + set reflectionTest(V) { const esValue = this !== null && this !== undefined ? this : globalObject; if (!exports.is(esValue)) { @@ -9369,10 +9416,10 @@ exports.install = function install(globalObject) { } V = conversions[\\"DOMString\\"](V, { - context: \\"Failed to set the 'ReflectionTest' property on 'Reflect': The provided value\\" + context: \\"Failed to set the 'reflectionTest' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"reflection\\", V); + esValue[impl][\\"reflectionTest\\"] = V; } get withUnderscore() { @@ -9382,8 +9429,7 @@ exports.install = function install(globalObject) { throw new TypeError(\\"Illegal invocation\\"); } - const value = this[impl].getAttributeNS(null, \\"with-underscore\\"); - return value === null ? \\"\\" : value; + return esValue[impl][\\"withUnderscore\\"]; } set withUnderscore(V) { @@ -9397,15 +9443,16 @@ exports.install = function install(globalObject) { context: \\"Failed to set the 'withUnderscore' property on 'Reflect': The provided value\\" }); - this[impl].setAttributeNS(null, \\"with-underscore\\", V); + esValue[impl][\\"withUnderscore\\"] = V; } } Object.defineProperties(Reflect.prototype, { - ReflectedBoolean: { enumerable: true }, - ReflectedDOMString: { enumerable: true }, - ReflectedLong: { enumerable: true }, - ReflectedUnsignedLong: { enumerable: true }, - ReflectionTest: { enumerable: true }, + reflectedBoolean: { enumerable: true }, + reflectedDOMString: { enumerable: true }, + reflectedLong: { enumerable: true }, + reflectedUnsignedLong: { enumerable: true }, + reflectedUSVStringURL: { enumerable: true }, + reflectionTest: { enumerable: true }, withUnderscore: { enumerable: true }, [Symbol.toStringTag]: { value: \\"Reflect\\", configurable: true } }); diff --git a/test/cases/Reflect.webidl b/test/cases/Reflect.webidl index 9268f83d..81cabe92 100644 --- a/test/cases/Reflect.webidl +++ b/test/cases/Reflect.webidl @@ -1,10 +1,11 @@ [Exposed=Window] interface Reflect { - [Reflect] attribute boolean ReflectedBoolean; - [FooBar, Reflect] attribute DOMString ReflectedDOMString; - [Reflect, FooBar] attribute long ReflectedLong; - [Reflect] attribute unsigned long ReflectedUnsignedLong; + [Reflect] attribute boolean reflectedBoolean; + [FooBar, Reflect] attribute DOMString reflectedDOMString; + [Reflect, FooBar] attribute long reflectedLong; + [Reflect] attribute unsigned long reflectedUnsignedLong; + [FooBar, ReflectURL] attribute USVString reflectedUSVStringURL; - [FooBar, Reflect=reflection] attribute DOMString ReflectionTest; + [FooBar, Reflect=reflection] attribute DOMString reflectionTest; [Reflect=with_underscore, FooBar] attribute DOMString withUnderscore; }; diff --git a/test/reflector.js b/test/reflector.js new file mode 100644 index 00000000..87cbaa55 --- /dev/null +++ b/test/reflector.js @@ -0,0 +1,64 @@ +"use strict"; + +module.exports.boolean = { + get(implObj, attrName) { + return `return ${implObj}.hasAttributeNS(null, "${attrName}");`; + }, + set(implObj, attrName) { + return ` + if (V) { + ${implObj}.setAttributeNS(null, "${attrName}", ""); + } else { + ${implObj}.removeAttributeNS(null, "${attrName}"); + } + `; + } +}; + +module.exports.DOMString = { + get(implObj, attrName) { + return ` + const value = ${implObj}.getAttributeNS(null, "${attrName}"); + return value === null ? "" : value; + `; + }, + set(implObj, attrName) { + return `${implObj}.setAttributeNS(null, "${attrName}", V);`; + } +}; + +module.exports.USVString = { + get(implObj, attrName) { + return ` + const value = ${implObj}.getAttributeNS(null, "${attrName}"); + return value === null ? "" : value; + `; + }, + set(implObj, attrName) { + return `${implObj}.setAttributeNS(null, "${attrName}", V);`; + } +}; + +module.exports.long = { + get(implObj, attrName) { + return ` + const value = parseInt(${implObj}.getAttributeNS(null, "${attrName}")); + return isNaN(value) || value < -2147483648 || value > 2147483647 ? 0 : value + `; + }, + set(implObj, attrName) { + return `${implObj}.setAttributeNS(null, "${attrName}", String(V));`; + } +}; + +module.exports["unsigned long"] = { + get(implObj, attrName) { + return ` + const value = parseInt(${implObj}.getAttributeNS(null, "${attrName}")); + return isNaN(value) || value < 0 || value > 2147483647 ? 0 : value + `; + }, + set(implObj, attrName) { + return `${implObj}.setAttributeNS(null, "${attrName}", String(V > 2147483647 ? 0 : V));`; + } +}; diff --git a/test/test.js b/test/test.js index 28db1cd7..092338d0 100644 --- a/test/test.js +++ b/test/test.js @@ -3,6 +3,7 @@ const fs = require("fs"); const path = require("path"); const Transformer = require(".."); +const reflector = require("./reflector"); const rootDir = path.resolve(__dirname, ".."); const casesDir = path.resolve(__dirname, "cases"); @@ -50,6 +51,35 @@ describe("with processors", () => { return ` return ${htmlConstructor}(globalObject, interfaceName); `; + }, + processReflect(idl, implObj) { + const reflectAttr = idl.extAttrs.find(attr => attr.name === "Reflect"); + const attrName = + reflectAttr && reflectAttr.rhs && reflectAttr.rhs.value.replace(/_/g, "-") || idl.name.toLowerCase(); + if (idl.idlType.idlType === "USVString") { + const reflectURL = idl.extAttrs.find(attr => attr.name === "ReflectURL"); + if (reflectURL) { + const whatwgURL = this.addImport("whatwg-url"); + return { + get: ` + const value = ${implObj}.getAttributeNS(null, "${attrName}"); + if (value === null) { + return ""; + } + const urlRecord = ${whatwgURL}.parseURL(value, { baseURL: "http://localhost:8080/" }); + return urlRecord === null ? conversions.USVString(value) : ${whatwgURL}.serializeURL(urlRecord); + `, + set: ` + ${implObj}.setAttributeNS(null, "${attrName}", V); + ` + }; + } + } + const reflect = reflector[idl.idlType.idlType]; + return { + get: reflect.get(implObj, attrName), + set: reflect.set(implObj, attrName) + }; } }); transformer.addSource(casesDir, implsDir); From 17dad725cfab4299c4b3775be58812f41a627ca9 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 23 Mar 2020 14:10:32 -0400 Subject: [PATCH 2/5] domenic fix 1 Co-Authored-By: Domenic Denicola --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8249c021..724c5f6e 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ Both hooks have the signature `(code) => replacementCode`, where: If either hook is omitted, then the code will not be replaced, i.e. the default is equivalent to `(code) => code`. -Both hooks also has a utility method that is accessible via `this`: +Both hooks also have a utility method that is accessible via `this`: - `addImport(path, [importedIdentifier])` utility to require external modules from the generated interface. This method accepts 2 parameters: `path` the relative or absolute path from the generated interface file, and an optional `importedIdentifier` the identifier to import. This method returns the local identifier from the imported path. From eaf28ebda167b7abc230cc568d3d012b655f03d2 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 23 Mar 2020 14:11:08 -0400 Subject: [PATCH 3/5] domenic fix 2 Co-Authored-By: Domenic Denicola --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 724c5f6e..e1634a6b 100644 --- a/README.md +++ b/README.md @@ -241,7 +241,7 @@ function processReflect(idl, implName) { ` }; } - throw new Error(`Unknown IDL type: ${idl.idlType.idlType}`); + throw new Error(`Not-yet-implemented IDL type for reflection: ${idl.idlType.idlType}`); } ``` From d80cd5cb6d168e0faa902e07aefa7f106bfef530 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 23 Mar 2020 14:11:18 -0400 Subject: [PATCH 4/5] domenic fix 3 Co-Authored-By: Domenic Denicola --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index e1634a6b..3dbb8bba 100644 --- a/README.md +++ b/README.md @@ -477,7 +477,7 @@ Notable missing features include: ## Nonstandard extended attributes -A non-standard extended attribute is baked in to webidl2js. +One non-standard extended attribute is baked in to webidl2js: ### `[WebIDL2JSValueAsUnsupported=value]` From bcd074ce31a4961ae68ba9422ecacd4757fb13b8 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Mon, 23 Mar 2020 14:13:15 -0400 Subject: [PATCH 5/5] Don't assume [Reflect] argument type --- README.md | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3dbb8bba..4b8b1f73 100644 --- a/README.md +++ b/README.md @@ -222,12 +222,8 @@ An example processor function that implements `boolean` IDL attribute reflection ```js function processReflect(idl, implName) { - // Determine the name of the content attribute being reflected as follows: - // - if [Reflect] is provided with a value, then the value is the content attribute. - // - otherwise, the content attribute is the name of the IDL attribute, lowercased. - const reflectAttr = idl.extAttrs.find(attr => attr.name === "Reflect"); - const attrName = - reflectAttr && reflectAttr.rhs && reflectAttr.rhs.value.replace(/_/g, "-") || idl.name.toLowerCase(); + // Assume the name of the reflected content attribute is the same as the IDL attribute, lowercased. + const attrName = idl.name.toLowerCase(); if (idl.idlType.idlType === "boolean") { return {