address some concerns from kschaaf

2025-02-25 18:55:30 -06:00 · 2017-06-29 16:47:17 -07:00
parent 3e14a1d812
commit 446538135e
8 changed files with 59 additions and 90 deletions
--- a/.eslintrc.json
+++ b/.eslintrc.json
@@ -1,6 +1,8 @@
 {
  "extends": "eslint:recommended",
-  "ecmaVersion": 6,
+  "parserOptions": {
+    "ecmaVersion": 6
+  },
  "rules": {
    "no-console": "off",
    "no-var": "error",
--- a/externs/polymer-closure-types.html
+++ b/externs/polymer-closure-types.html
@@ -154,4 +154,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   * }}
   */
  let TemplatizeOptions;
+
+  /** @type {Polymer_PropertyEffects} */
+  Polymer_PropertyEffects.prototype.__dataHost;
 </script>
--- a/externs/polymer-externs.js
+++ b/externs/polymer-externs.js
@@ -64,20 +64,18 @@ Polymer.sanitizeDOMValue;
 */
 function JSCompiler_renameProperty(string, obj) {}

-/** @type {Object} */
-Polymer.telemetry;
+/** @record */
+function PolymerTelemetry(){}
+/** @type {number} */
+PolymerTelemetry.instanceCount;
+/** @type {Array<HTMLElement>} */
+PolymerTelemetry.registrations;
+/** @type {function(HTMLElement)} */
+PolymerTelemetry._regLog;
+/** @type {function(HTMLElement)} */
+PolymerTelemetry.register;
+/** @type {function(HTMLElement)} */
+PolymerTelemetry.dumpRegistrations;;

-/** @type {Object} */
-Polymer_PropertyEffects.prototype.__computeEffects;
-/** @type {Object} */
-Polymer_PropertyEffects.prototype.__reflectEffects;
-/** @type {Object} */
-Polymer_PropertyEffects.prototype.__notifyEffects;
-/** @type {Object} */
-Polymer_PropertyEffects.prototype.__propagateEffects;
-/** @type {Object} */
-Polymer_PropertyEffects.prototype.__observeEffects;
-/** @type {Object} */
-Polymer_PropertyEffects.prototype.__readOnly;
-/** @type {Polymer_PropertyEffects} */
-Polymer_PropertyEffects.prototype.__dataHost;
+/** @type {PolymerTelemetry} */
+Polymer.telemetry;
--- a/gulpfile.js
+++ b/gulpfile.js
@@ -37,7 +37,7 @@ const PolymerProject = polymer.PolymerProject;

 const {Transform} = require('stream');

-class OldNameStream extends Transform {
+class BackfillStream extends Transform {
  constructor(fileList) {
    super({objectMode: true});
    this.fileList = fileList;
@@ -63,45 +63,15 @@ class OldNameStream extends Transform {
  }
 }

-class Log extends Transform {
-  constructor(prefix = '') {
-    super({objectMode: true});
-    this.prefix = prefix;
-  }
-  _transform(file, enc, cb) {
-    console.log(this.prefix, file.path);
-    cb(null, file);
-  }
-}
-
-class Uniq extends Transform {
-  constructor() {
-    super({ objectMode: true });
-    this.map = {};
-  }
-  _transform(file, enc, cb) {
-    this.map[file.path] = file;
-    cb();
-  }
-  _flush(done) {
-    for (let filePath in this.map) {
-      let file = this.map[filePath];
-      this.push(file);
-    }
-    done();
-  }
-}
-
 let CLOSURE_LINT_ONLY = false;
-let EXPECTED_WARNING_COUNT = 498;

 let firstImportFinder = dom5.predicates.AND(dom5.predicates.hasTagName('link'), dom5.predicates.hasAttrValue('rel', 'import'));

-class AddClosureOnlyImport extends Transform {
-  constructor(fileName, dependency) {
+class AddClosureTypeImport extends Transform {
+  constructor(entryFileName, typeFileName) {
    super({objectMode: true});
-    this.target = path.resolve(fileName);
-    this.importPath = path.resolve(dependency);
+    this.target = path.resolve(entryFileName);
+    this.importPath = path.resolve(typeFileName);
  }
  _transform(file, enc, cb) {
    if (file.path === this.target) {
@@ -125,13 +95,13 @@ gulp.task('clean', () => del([DIST_DIR, 'closure.log']));

 gulp.task('closure', ['clean'], () => {

-  let entry, splitRx, joinRx, closureImportAdder;
+  let entry, splitRx, joinRx, addClosureTypes;

  function config(path) {
    entry = path;
    joinRx = new RegExp(path.split('/').join('\\/'));
    splitRx = new RegExp(joinRx.source + '_script_\\d+\\.js$');
-    closureImportAdder = new AddClosureOnlyImport(entry, 'externs/polymer-closure-types.html');
+    addClosureTypes = new AddClosureTypeImport(entry, 'externs/polymer-closure-types.html');
  }

  config('polymer.html');
@@ -144,20 +114,17 @@ gulp.task('closure', ['clean'], () => {
      'bower_components/shadycss/custom-style-interface.html'
    ],
    extraDependencies: [
-      closureImportAdder.importPath
+      addClosureTypes.importPath,
+      'externs/closure-types.js'
    ]
  });

  function closureLintLogger(log) {
    let chalk = require('chalk');
-    let result = log.split(/\n/).slice(-2)[0];
-    let warnings = result.match(/(\d+) warning/);
    // write out log to use with diffing tools later
    fs.writeFileSync('closure.log', chalk.stripColor(log));
-    if (warnings && Number(warnings[1]) > EXPECTED_WARNING_COUNT) {
-      console.error(chalk.red(`closure linting: actual warning count ${warnings[1]} greater than expected warning count ${EXPECTED_WARNING_COUNT}`));
-      process.exit(1);
-    }
+    console.error(log);
+    process.exit(-1);
  }

  let closurePluginOptions;
@@ -197,7 +164,7 @@ gulp.task('closure', ['clean'], () => {

  const closurePipeline = lazypipe()
    .pipe(() => closureStream)
-    .pipe(() => new OldNameStream(closureStream.fileList_))
+    .pipe(() => new BackfillStream(closureStream.fileList_))

  // process source files in the project
  const sources = project.sources();
@@ -210,9 +177,8 @@ gulp.task('closure', ['clean'], () => {

  const splitter = new polymer.HtmlSplitter();
  return mergedFiles
-    .pipe(closureImportAdder)
+    .pipe(addClosureTypes)
    .pipe(project.bundler())
-    .pipe(new Uniq())
    .pipe(splitter.split())
    .pipe(gulpif(splitRx, closurePipeline()))
    .pipe(splitter.rejoin())
@@ -260,7 +226,6 @@ gulp.task('estimate-size', ['clean'], () => {
  return mergedFiles
    .pipe(project.bundler())
    .pipe(gulpif(/polymer\.html$/, bundlePipe()))
-    .pipe(new Uniq())
    .pipe(gulpif(/polymer\.html$/, size({ title: 'bundled size', gzip: true, showTotal: false, showFiles: true })))
    // write to the bundled folder
    .pipe(gulp.dest(BUNDLED_DIR))
--- a/lib/mixins/element-mixin.html
+++ b/lib/mixins/element-mixin.html
@@ -788,6 +788,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
    registrations: [],
    /**
     * @param {!PolymerElementConstructor} prototype Element prototype to log
+     * @this {this}
     * @private
     */
    _regLog: function(prototype) {
@@ -796,8 +797,8 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
    /**
     * Registers a class prototype for telemetry purposes.
     * @param {HTMLElement} prototype Element prototype to register
-     * @protected
     * @this {this}
+     * @protected
     */
    register: function(prototype) {
      this.registrations.push(prototype);
--- a/lib/mixins/property-effects.html
+++ b/lib/mixins/property-effects.html
@@ -236,7 +236,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   */
  function runNotifyEffects(inst, notifyProps, props, oldProps, hasPaths) {
    // Notify
-    let fxs = inst.__notifyEffects;
+    let fxs = inst[TYPES.NOTIFY];
    let notified;
    let id = dedupeId++;
    // Try normal notify effects; if none, fall back to try path notification
@@ -352,7 +352,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
      value = event.target[fromProp];
    }
    value = negate ? !value : value;
-    if (!inst.__readOnly || !inst.__readOnly[toPath]) {
+    if (!inst[TYPES.READ_ONLY] || !inst[TYPES.READ_ONLY][toPath]) {
      if (inst._setPendingPropertyOrPath(toPath, value, true, Boolean(fromPath))
        && (!detail || !detail.queueProperty)) {
        inst._invalidateProperties();
@@ -397,7 +397,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
   * @private
   */
  function runComputedEffects(inst, changedProps, oldProps, hasPaths) {
-    let computeEffects = inst.__computeEffects;
+    let computeEffects = inst[TYPES.COMPUTE];
    if (computeEffects) {
      let inputProps = changedProps;
      while (runEffects(inst, computeEffects, inputProps, oldProps, hasPaths)) {
@@ -592,7 +592,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
      // Property binding
      let prop = binding.target;
      if (node.__dataHasAccessor && node.__dataHasAccessor[prop]) {
-        if (!node.__readOnly || !node.__readOnly[prop]) {
+        if (!node[TYPES.READ_ONLY] || !node[TYPES.READ_ONLY][prop]) {
          if (node._setPendingProperty(prop, value)) {
            inst._enqueueClient(node);
          }
@@ -1195,7 +1195,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @param {Object} props Properties to initialize on the instance
       */
      _initializeInstanceProperties(props) {
-        let readOnly = this.__readOnly;
+        let readOnly = this[TYPES.READ_ONLY];
        for (let prop in props) {
          if (!readOnly || !readOnly[prop]) {
            this.__dataPending = this.__dataPending || {};
@@ -1455,7 +1455,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
          // All changes go into pending property bag, passed to _propertiesChanged
          this.__dataPending[property] = value;
          // Track properties that should notify separately
-          if (isPath || (this.__notifyEffects && this.__notifyEffects[property])) {
+          if (isPath || (this[TYPES.NOTIFY] && this[TYPES.NOTIFY][property])) {
            this.__dataToNotify = this.__dataToNotify || {};
            this.__dataToNotify[property] = shouldNotify;
          }
@@ -1578,7 +1578,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       */
      setProperties(props, setReadOnly) {
        for (let path in props) {
-          if (setReadOnly || !this.__readOnly || !this.__readOnly[path]) {
+          if (setReadOnly || !this[TYPES.READ_ONLY] || !this[TYPES.READ_ONLY][path]) {
            //TODO(kschaaf): explicitly disallow paths in setProperty?
            // wildcard observers currently only pass the first changed path
            // in the `info` object, and you could do some odd things batching
@@ -1642,9 +1642,9 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
        // Flush clients
        this._flushClients();
        // Reflect properties
-        runEffects(this, this.__reflectEffects, changedProps, oldProps, hasPaths);
+        runEffects(this, this[TYPES.REFLECT], changedProps, oldProps, hasPaths);
        // Observe properties
-        runEffects(this, this.__observeEffects, changedProps, oldProps, hasPaths);
+        runEffects(this, this[TYPES.OBSERVE], changedProps, oldProps, hasPaths);
        // Notify properties to host
        if (notifyProps) {
          runNotifyEffects(this, notifyProps, changedProps, oldProps, hasPaths);
@@ -1668,10 +1668,10 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @protected
       */
      _propagatePropertyChanges(changedProps, oldProps, hasPaths) {
-        if (this.__propagateEffects) {
-          runEffects(this, this.__propagateEffects, changedProps, oldProps, hasPaths);
+        if (this[TYPES.PROPAGATE]) {
+          runEffects(this, this[TYPES.PROPAGATE], changedProps, oldProps, hasPaths);
        }
-        let templateInfo = /** @type {TemplateInfo} */(this.__templateInfo);
+        let templateInfo = this.__templateInfo;
        while (templateInfo) {
          runEffects(this, templateInfo.propertyEffects, changedProps, oldProps,
            hasPaths, templateInfo.nodeList);
@@ -1740,7 +1740,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @public
      */
      notifySplices(path, splices) {
-        let /** PathInfo */ info = {path: ''};
+        let info = {path: ''};
        let array = /** @type {Array} */(Polymer.Path.get(this, path, info));
        notifySplices(this, array, info.path, splices);
      }
@@ -1792,7 +1792,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
        if (root) {
          Polymer.Path.set(root, path, value);
        } else {
-          if (!this.__readOnly || !this.__readOnly[/** @type {string} */(path)]) {
+          if (!this[TYPES.READ_ONLY] || !this[TYPES.READ_ONLY][/** @type {string} */(path)]) {
            if (this._setPendingPropertyOrPath(path, value, true)) {
              this._invalidateProperties();
            }
@@ -1815,7 +1815,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @public
       */
      push(path, ...items) {
-        let /** PathInfo */ info = {path: ''};
+        let info = {path: ''};
        let array = /** @type {Array}*/(Polymer.Path.get(this, path, info));
        let len = array.length;
        let ret = array.push(...items);
@@ -1839,7 +1839,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @public
       */
      pop(path) {
-        let /** PathInfo */ info = {path: ''};
+        let info = {path: ''};
        let array = /** @type {Array} */(Polymer.Path.get(this, path, info));
        let hadLength = Boolean(array.length);
        let ret = array.pop();
@@ -1867,7 +1867,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @public
       */
      splice(path, start, deleteCount, ...items) {
-        let /** PathInfo */ info = {path : ''};
+        let info = {path : ''};
        let array = /** @type {Array} */(Polymer.Path.get(this, path, info));
        // Normalize fancy native splice handling of crazy start values
        if (start < 0) {
@@ -1899,7 +1899,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @public
       */
      shift(path) {
-        let /** PathInfo */ info = {path: ''};
+        let info = {path: ''};
        let array = /** @type {Array} */(Polymer.Path.get(this, path, info));
        let hadLength = Boolean(array.length);
        let ret = array.shift();
@@ -1924,7 +1924,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @public
       */
      unshift(path, ...items) {
-        let /** PathInfo */ info = {path: ''};
+        let info = {path: ''};
        let array = /** @type {Array} */(Polymer.Path.get(this, path, info));
        let ret = array.unshift(...items);
        if (items.length) {
@@ -1950,7 +1950,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
        let propPath;
        if (arguments.length == 1) {
          // Get value if not supplied
-          let /** PathInfo */ info = {path: ''};
+          let info = {path: ''};
          value = Polymer.Path.get(this, path, info);
          propPath = info.path;
        } else if (Array.isArray(path)) {
--- a/lib/mixins/template-stamp.html
+++ b/lib/mixins/template-stamp.html
@@ -349,7 +349,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
       * @param {!TemplateInfo} templateInfo Template metadata for current template
       * @param {!NodeInfo} nodeInfo Node metadata for current template.
       * @param {string} name Attribute name
-       * @param {*} value Attribute value
+       * @param {string} value Attribute value
       * @return {boolean} `true` if the visited node added node-specific
       *   metadata to `nodeInfo`
       */
@@ -366,7 +366,7 @@ subject to an additional IP rights grant found at http://polymer.github.io/PATEN
        }
        // static id
        else if (name === 'id') {
-          nodeInfo.id = String(value);
+          nodeInfo.id = value;
          return true;
        }
        return false;
--- a/package.json
+++ b/package.json
@@ -11,8 +11,8 @@
    "@polymer/gen-closure-declarations": "0.0.5",
    "@webcomponents/shadycss": "^1.0.0",
    "@webcomponents/webcomponentsjs": "^1.0.0",
-    "babel-preset-babili": "0.1.2",
-    "del": "^2.2.1",
+    "babel-preset-babili": "^0.1.4",
+    "del": "^3.0.0",
    "dom5": "^1.3.1",
    "eslint-plugin-html": "^2.0.1",
    "google-closure-compiler": "^20170521.0.0",