Add proper reflection support through processor (#161)

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
2a2a099. Since this commit moves the
reflection code out of webidl2js, the bug now naturally disappears.

Closes #15. Closes #93.

Co-authored-by: ExE Boss <[email protected]>
Co-authored-by: Domenic Denicola <[email protected]>

Author: 3 people

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 have 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,65 @@ 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) {
+  // 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 {
+      get: `return ${implName}.hasAttributeNS(null, "${attrName}");`,
+      set: `
+        if (V) {
+          ${implName}.setAttributeNS(null, "${attrName}", "");
+        } else {
+          ${implName}.removeAttributeNS(null, "${attrName}");
+        }
+      `
+    };
+  }
+  throw new Error(`Not-yet-implemented IDL type for reflection: ${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 +473,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.
+One non-standard extended attribute is baked in to webidl2js:
 
 ### `[WebIDL2JSValueAsUnsupported=value]`
 

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)) {

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()}

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 = `

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);
   }
 }
 

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";
           }

lib/reflector.js

@@ -19,6 +19,7 @@ class Transformer {
       implSuffix: opts.implSuffix,
       processCEReactions: opts.processCEReactions,
       processHTMLConstructor: opts.processHTMLConstructor,
+      processReflect: opts.processReflect,
       options: {
         suppressErrors: Boolean(opts.suppressErrors)
       }

lib/transformer.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";
         }

lib/types.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")`;