Merge branch 'master' into refactor/rename-imported-symbols

Author: ExE-Boss

README.md

@@ -56,7 +56,7 @@ class SomeInterface {
       context: "Failed to execute 'add' on 'SomeInterface': parameter 2"
     });
 
-    return this[impl].add(...args);
+    return this[implSymbol].add(...args);
   }
 }
 
@@ -68,11 +68,11 @@ Object.defineProperties(SomeInterface.prototype, {
 exports.create = (globalObject, constructorArgs = [], privateData = {}) => {
   const ctor = globalObject[ctorRegistry].SomeInterface;
   const obj = Object.create(ctor.prototype);
-  obj[impl] = new Impl(constructorArgs, privateData);
+  obj[implSymbol] = new Impl(constructorArgs, privateData);
   return obj;
 };
 
-exports.is = obj => obj && obj[impl] instanceof Impl;
+exports.is = obj => obj && obj[implSymbol] instanceof Impl;
 ```
 
 The above is a simplification of the actual generated code, but should give you some idea of what's going on. We bring your attention to a few points:
@@ -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[implSymbol]", { 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

@@ -495,7 +495,7 @@ class Interface {
     this.requires.addRaw(`{ implSymbol, ctorRegistrySymbol }`, "utils");
 
     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)
       }
@@ -256,6 +257,8 @@ class Transformer {
   _prettify(source) {
     return prettier.format(source, {
       printWidth: 120,
+      trailingComma: "none",
+      arrowParens: "avoid",
       parser: "babel"
     });
   }

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")`;