Skip to content

Commit

Permalink
address some concerns from kschaaf
Browse files Browse the repository at this point in the history
  • Loading branch information
dfreedm committed Jun 29, 2017
1 parent 3e14a1d commit 4465381
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 90 deletions.
4 changes: 3 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
{
"extends": "eslint:recommended",
"ecmaVersion": 6,
"parserOptions": {
"ecmaVersion": 6
},
"rules": {
"no-console": "off",
"no-var": "error",
Expand Down
3 changes: 3 additions & 0 deletions externs/polymer-closure-types.html
Original file line number Diff line number Diff line change
Expand Up @@ -154,4 +154,7 @@
* }}
*/
let TemplatizeOptions;

/** @type {Polymer_PropertyEffects} */
Polymer_PropertyEffects.prototype.__dataHost;
</script>
30 changes: 14 additions & 16 deletions externs/polymer-externs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
61 changes: 13 additions & 48 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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');
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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())
Expand Down Expand Up @@ -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))
Expand Down
3 changes: 2 additions & 1 deletion lib/mixins/element-mixin.html
Original file line number Diff line number Diff line change
Expand Up @@ -788,6 +788,7 @@
registrations: [],
/**
* @param {!PolymerElementConstructor} prototype Element prototype to log
* @this {this}
* @private
*/
_regLog: function(prototype) {
Expand All @@ -796,8 +797,8 @@
/**
* 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);
Expand Down
40 changes: 20 additions & 20 deletions lib/mixins/property-effects.html
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@
*/
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
Expand Down Expand Up @@ -352,7 +352,7 @@
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();
Expand Down Expand Up @@ -397,7 +397,7 @@
* @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)) {
Expand Down Expand Up @@ -592,7 +592,7 @@
// 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);
}
Expand Down Expand Up @@ -1195,7 +1195,7 @@
* @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 || {};
Expand Down Expand Up @@ -1455,7 +1455,7 @@
// 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;
}
Expand Down Expand Up @@ -1578,7 +1578,7 @@
*/
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
Expand Down Expand Up @@ -1642,9 +1642,9 @@
// 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);
Expand All @@ -1668,10 +1668,10 @@
* @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);
Expand Down Expand Up @@ -1740,7 +1740,7 @@
* @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);
}
Expand Down Expand Up @@ -1792,7 +1792,7 @@
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();
}
Expand All @@ -1815,7 +1815,7 @@
* @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);
Expand All @@ -1839,7 +1839,7 @@
* @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();
Expand Down Expand Up @@ -1867,7 +1867,7 @@
* @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) {
Expand Down Expand Up @@ -1899,7 +1899,7 @@
* @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();
Expand All @@ -1924,7 +1924,7 @@
* @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) {
Expand All @@ -1950,7 +1950,7 @@
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)) {
Expand Down
4 changes: 2 additions & 2 deletions lib/mixins/template-stamp.html
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@
* @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`
*/
Expand All @@ -366,7 +366,7 @@
}
// static id
else if (name === 'id') {
nodeInfo.id = String(value);
nodeInfo.id = value;
return true;
}
return false;
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 4465381

Please sign in to comment.