Address review comments

Co-authored-by: Timothy Gu <timothygu99@gmail.com>

lib/CSSStyleDeclaration-impl.js

@@ -7,25 +7,26 @@ var CSSOM = require('cssom');
 var allProperties = require('./allProperties');
 var allExtraProperties = require('./allExtraProperties');
 var implementedProperties = require('./implementedProperties');
-var { dashedToCamelCase } = require('./parsers');
+var { cssPropertyToIDLAttribute } = require('./parsers');
 var getBasicPropertyDescriptor = require('./utils/getBasicPropertyDescriptor');
 const idlUtils = require('./utils.js');
 
-class CSSStyleDeclaration {
+class CSSStyleDeclarationImpl {
   /**
    * @constructor
    * @see http://www.w3.org/TR/DOM-Level-2-Style/css.html#CSS-CSSStyleDeclaration
+   *
+   * @param {object} globalObject
+   * @param {*[]} args
+   * @param {object} privateData
+   * @param {((cssText: string) => void) | null} [privateData.onChangeCallback]
    */
-  constructor(globalObject, args, { onChangeCallback }) {
+  constructor(globalObject, args, { onChangeCallback = null }) {
     this._globalObject = globalObject;
     this._values = {};
     this._importants = {};
     this._length = 0;
-    this._onChange =
-      onChangeCallback ||
-      function() {
-        return;
-      };
+    this._onChange = onChangeCallback || (() => {});
   }
 
   /**
@@ -150,7 +151,7 @@ class CSSStyleDeclaration {
   }
 }
 
-Object.defineProperties(CSSStyleDeclaration.prototype, {
+Object.defineProperties(CSSStyleDeclarationImpl.prototype, {
   cssText: {
     get: function() {
       var properties = [];
@@ -224,22 +225,30 @@ Object.defineProperties(CSSStyleDeclaration.prototype, {
   },
 });
 
-require('./properties')(CSSStyleDeclaration.prototype);
+require('./properties')(CSSStyleDeclarationImpl.prototype);
 
 allProperties.forEach(function(property) {
   if (!implementedProperties.has(property)) {
     var declaration = getBasicPropertyDescriptor(property);
-    Object.defineProperty(CSSStyleDeclaration.prototype, property, declaration);
-    Object.defineProperty(CSSStyleDeclaration.prototype, dashedToCamelCase(property), declaration);
+    Object.defineProperty(CSSStyleDeclarationImpl.prototype, property, declaration);
+    Object.defineProperty(
+      CSSStyleDeclarationImpl.prototype,
+      cssPropertyToIDLAttribute(property),
+      declaration
+    );
   }
 });
 
 allExtraProperties.forEach(function(property) {
   if (!implementedProperties.has(property)) {
     var declaration = getBasicPropertyDescriptor(property);
-    Object.defineProperty(CSSStyleDeclaration.prototype, property, declaration);
-    Object.defineProperty(CSSStyleDeclaration.prototype, dashedToCamelCase(property), declaration);
+    Object.defineProperty(CSSStyleDeclarationImpl.prototype, property, declaration);
+    Object.defineProperty(
+      CSSStyleDeclarationImpl.prototype,
+      cssPropertyToIDLAttribute(property),
+      declaration
+    );
   }
 });
 
-exports.implementation = CSSStyleDeclaration;
+exports.implementation = CSSStyleDeclarationImpl;

lib/CSSStyleDeclaration.test.js

@@ -5,17 +5,16 @@ var CSSStyleDeclaration = require('../index.js');
 var allProperties = require('./allProperties');
 var allExtraProperties = require('./allExtraProperties');
 var implementedProperties = require('./implementedProperties');
-var parsers = require('./parsers');
+var { cssPropertyToIDLAttribute } = require('./parsers');
 
 var dashedProperties = [...allProperties, ...allExtraProperties];
-var allowedProperties = dashedProperties.map(parsers.dashedToCamelCase);
-allowedProperties.push('cssFloat');
-implementedProperties = Array.from(implementedProperties).map(parsers.dashedToCamelCase);
+var allowedProperties = ['cssFloat', ...dashedProperties.map(p => cssPropertyToIDLAttribute(p))];
+implementedProperties = Array.from(implementedProperties, p => cssPropertyToIDLAttribute(p));
 var invalidProperties = implementedProperties.filter(prop => !allowedProperties.includes(prop));
 
 describe('CSSStyleDeclaration', () => {
   test('has only valid properties implemented', () => {
-    expect(invalidProperties.length).toEqual(0);
+    expect(invalidProperties).toEqual([]);
   });
 
   test('has all properties', () => {

lib/parsers.js

@@ -447,22 +447,35 @@ exports.parseKeyword = function parseKeyword(val, valid_keywords) {
   return undefined;
 };
 
-// utility to translate from border-width to borderWidth
-var dashedToCamelCase = function(dashed) {
-  var i;
-  var camel = '';
-  var nextCap = false;
-  for (i = 0; i < dashed.length; i++) {
-    if (dashed[i] !== '-') {
-      camel += nextCap ? dashed[i].toUpperCase() : dashed[i];
-      nextCap = false;
+/**
+ * utility to translate from border-width to borderWidth
+ *
+ * @param {string} property
+ * @param {boolean} [lowercaseFirst]
+ * @see https://drafts.csswg.org/cssom/#css-property-to-idl-attribute
+ */
+function cssPropertyToIDLAttribute(property, lowercaseFirst = false) {
+  let output = '';
+  let uppercaseNext = false;
+
+  if (lowercaseFirst) {
+    property = property.substring(1);
+  }
+
+  for (const c of property) {
+    if (c === '-') {
+      uppercaseNext = true;
+    } else if (uppercaseNext) {
+      uppercaseNext = false;
+      output += c.toUpperCase();
     } else {
-      nextCap = true;
+      output += c;
     }
   }
-  return camel;
-};
-exports.dashedToCamelCase = dashedToCamelCase;
+
+  return output;
+}
+exports.cssPropertyToIDLAttribute = cssPropertyToIDLAttribute;
 
 var is_space = /\s/;
 var opening_deliminators = ['"', "'", '('];
@@ -566,7 +579,7 @@ exports.shorthandSetter = function(property, shorthand_for) {
     Object.keys(obj).forEach(function(subprop) {
       // in case subprop is an implicit property, this will clear
       // *its* subpropertiesX
-      var camel = dashedToCamelCase(subprop);
+      var camel = cssPropertyToIDLAttribute(subprop);
       this[camel] = obj[subprop];
       // in case it gets translated into something else (0 -> 0px)
       obj[subprop] = this[camel];
@@ -708,15 +721,15 @@ exports.subImplicitSetter = function(prefix, part, isValid, parser) {
   };
 };
 
-var camel_to_dashed = /[A-Z]/g;
-var first_segment = /^([^-]+)-/;
-var vendor_prefixes = ['o', 'moz', 'ms', 'webkit'];
-exports.camelToDashed = function(camel_case) {
-  var match;
-  var dashed = camel_case.replace(camel_to_dashed, '-$&').toLowerCase();
-  match = dashed.match(first_segment);
-  if (match && vendor_prefixes.indexOf(match[1]) !== -1) {
-    dashed = '-' + dashed;
-  }
-  return dashed;
+const camel_to_dashed = /[A-Z]/g;
+
+/**
+ * @param {string} attribute
+ * @param {boolean} [dashPrefix]
+ * @see https://drafts.csswg.org/cssom/#idl-attribute-to-css-property
+ */
+exports.idlAttributeToCSSProperty = (attribute, dashPrefix = false) => {
+  let output = dashPrefix ? '-' : '';
+  output += attribute.replace(camel_to_dashed, '-$&').toLowerCase();
+  return output;
 };

lib/parsers.test.js

@@ -116,7 +116,7 @@ describe('parseAngle', () => {
 describe('parseKeyword', () => {
   it.todo('test');
 });
-describe('dashedToCamelCase', () => {
+describe('cssPropertyToIDLAttribute', () => {
   it.todo('test');
 });
 describe('shorthandParser', () => {
@@ -134,6 +134,6 @@ describe('implicitSetter', () => {
 describe('subImplicitSetter', () => {
   it.todo('test');
 });
-describe('camelToDashed', () => {
+describe('idlAttributeToCSSProperty', () => {
   it.todo('test');
 });

scripts/convert-idl.js

@@ -6,22 +6,23 @@ const Transformer = require('webidl2js');
 
 const allProperties = require('../lib/allProperties.js');
 const allExtraProperties = require('../lib/allExtraProperties.js');
-const parsers = require('../lib/parsers.js');
+const { cssPropertyToIDLAttribute } = require('../lib/parsers.js');
 
 const srcDir = path.resolve(__dirname, '../src');
 const implDir = path.resolve(__dirname, '../lib');
 const outputDir = implDir;
 
-const propertyNames = Array.from(allProperties)
-  .concat(
-    Array.from(allExtraProperties).filter(prop => prop !== 'css-float' && !allProperties.has(prop))
-  )
-  .sort();
+const propertyNames = [
+  ...allProperties,
+  ...Array.from(allExtraProperties).filter(prop => {
+    return prop !== 'css-float' && !allProperties.has(prop);
+  }),
+].sort();
 
-const camelCasedAttributes = propertyNames.map(n => parsers.dashedToCamelCase(n));
+const camelCasedAttributes = propertyNames.map(n => cssPropertyToIDLAttribute(n));
 const webkitCasedAttributes = propertyNames
   .filter(n => n.startsWith('-webkit-'))
-  .map(n => parsers.dashedToCamelCase(n.substring(1)));
+  .map(n => cssPropertyToIDLAttribute(n, true));
 const dashedAttributes = propertyNames.filter(n => n.includes('-'));
 
 {
@@ -34,13 +35,11 @@ const dashedAttributes = propertyNames.filter(n => n.includes('-'));
     }
   );
 
-  const dateToday = new Date();
   genIDL.write(
-    `// autogenerated - ${dateToday.getMonth() +
-      1}/${dateToday.getDate()}/${dateToday.getFullYear()}
+    `// autogenerated by scripts/convert-idl.js. do not edit! ${new Date().toISOString()}
 
 partial interface CSSStyleDeclaration {
-  // https://drafts.csswg.org/cssom/#ref-for-cssstyledeclaration④
+  // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-camel_cased_attribute
 `
   );
 
@@ -50,7 +49,7 @@ partial interface CSSStyleDeclaration {
   }
 
   genIDL.write(`
-  // https://drafts.csswg.org/cssom/#ref-for-cssstyledeclaration⑤
+  // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-webkit_cased_attribute
 `);
 
   for (const webkitCasedAttribute of webkitCasedAttributes) {
@@ -59,7 +58,7 @@ partial interface CSSStyleDeclaration {
   }
 
   genIDL.write(`
-  // https://drafts.csswg.org/cssom/#ref-for-cssstyledeclaration⑥
+  // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-dashed_attribute
 `);
 
   for (const dashedAttribute of dashedAttributes) {

scripts/download_latest_properties.js

@@ -23,7 +23,7 @@ var path = require('path');
 
 var request = require('request');
 
-const { camelToDashed } = require('../lib/parsers');
+const { idlAttributeToCSSProperty } = require('../lib/parsers');
 
 var url = 'https://www.w3.org/Style/CSS/all-properties.en.json';
 
@@ -69,7 +69,7 @@ request(url, function(error, response, body) {
     out_file.write('/*\n *\n * https://www.w3.org/Style/CSS/all-properties.en.html\n */\n\n');
     out_file.write(
       'module.exports = new Set(' +
-        JSON.stringify(CSSpropertyNames.map(camelToDashed), null, 2) +
+        JSON.stringify(CSSpropertyNames.map(idlAttributeToCSSProperty), null, 2) +
         ');\n'
     );
 

scripts/generate_implemented_properties.js

@@ -4,12 +4,14 @@ const fs = require('fs');
 const path = require('path');
 const t = require('babel-types');
 const generate = require('babel-generator').default;
-const camelToDashed = require('../lib/parsers').camelToDashed;
+const { idlAttributeToCSSProperty } = require('../lib/parsers');
 
 const dashedProperties = fs
   .readdirSync(path.resolve(__dirname, '../lib/properties'))
-  .filter(propertyFile => propertyFile.substr(-3) === '.js')
-  .map(propertyFile => camelToDashed(propertyFile.replace('.js', '')));
+  .filter(propertyFile => propertyFile.slice(-3) === '.js')
+  .map(propertyFile => {
+    return idlAttributeToCSSProperty(propertyFile.slice(0, -3), propertyFile.startsWith('webkit'));
+  });
 
 const out_file = fs.createWriteStream(path.resolve(__dirname, '../lib/implementedProperties.js'), {
   encoding: 'utf-8',

scripts/generate_properties.js

@@ -8,7 +8,7 @@ var generate = require('babel-generator').default;
 var traverse = require('babel-traverse').default;
 var resolve = require('resolve');
 
-var camelToDashed = require('../lib/parsers').camelToDashed;
+var { idlAttributeToCSSProperty } = require('../lib/parsers');
 
 var basename = path.basename;
 var dirname = path.dirname;
@@ -254,7 +254,7 @@ parsedFiles.forEach(function(file) {
 });
 var propertyDefinitions = [];
 parsedFiles.forEach(function(file) {
-  var dashed = camelToDashed(file.property);
+  var dashed = idlAttributeToCSSProperty(file.property);
   propertyDefinitions.push(
     t.objectProperty(
       t.identifier(file.property),

src/CSSStyleDeclaration.webidl

@@ -14,3 +14,5 @@ interface CSSStyleDeclaration {
   readonly attribute CSSRule? parentRule;
   [CEReactions] attribute [TreatNullAs=EmptyString] CSSOMString cssFloat;
 };
+
+// Additional partial interfaces are generated by `scripts/convert-idl.js`.