-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2.x] Add legacy-data-mixin as 1.x->2.x migration aide. Fixes #5262. #5266
Changes from all commits
c46a38d
4ec63a4
2214b9e
7fc9564
b3bbd21
a470e45
6818554
08e13f2
c550e9e
61d1143
b856b89
5992774
d44969a
ed6deea
763e40d
7f2fcb1
55e9dfd
509b73f
75cfcb8
9f65049
6e4f62d
d91ac93
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,171 @@ | ||
<!-- | ||
@license | ||
Copyright (c) 2017 The Polymer Project Authors. All rights reserved. | ||
This code may only be used under the BSD style license found at http://polymer.github.io/LICENSE.txt | ||
The complete set of authors may be found at http://polymer.github.io/AUTHORS.txt | ||
The complete set of contributors may be found at http://polymer.github.io/CONTRIBUTORS.txt | ||
Code distributed by Google as part of the polymer project is also | ||
subject to an additional IP rights grant found at http://polymer.github.io/PATENTS.txt | ||
--> | ||
|
||
<link rel="import" href="class.html"> | ||
<link rel="import" href="../../polymer.html"> | ||
<link rel="import" href="../utils/mixin.html"> | ||
<link rel="import" href="../utils/templatize.html"> | ||
|
||
<script> | ||
(function() { | ||
'use strict'; | ||
|
||
const UndefinedArgumentError = class extends Error { | ||
constructor(message, arg) { | ||
super(message); | ||
this.arg = arg; | ||
this.name = this.constructor.name; | ||
// Affordances for ensuring instanceof works after babel ES5 compilation | ||
// TODO(kschaaf): Remove after polymer CLI updates to newer Babel that | ||
// sets the constructor/prototype correctly for subclassed builtins | ||
this.constructor = UndefinedArgumentError; | ||
this.__proto__ = UndefinedArgumentError.prototype; | ||
} | ||
}; | ||
|
||
/** | ||
* Wraps effect functions to catch `UndefinedArgumentError`s and warn. | ||
* | ||
* @param {Object=} effect Effect metadata object | ||
* @param {Object=} fnName Name of user function, if known | ||
* @return {?Object} Effect metadata object | ||
*/ | ||
function wrapEffect(effect, fnName) { | ||
if (effect && effect.fn) { | ||
const fn = effect.fn; | ||
effect.fn = function() { | ||
try { | ||
fn.apply(this, arguments); | ||
} catch (e) { | ||
if (e instanceof UndefinedArgumentError) { | ||
console.warn(`Argument '${e.arg}'${fnName ?` for method '${fnName}'` : ''} was undefined. Ensure it has an undefined check.`); | ||
} else { | ||
throw e; | ||
} | ||
} | ||
}; | ||
} | ||
return effect; | ||
} | ||
|
||
/** | ||
* Mixin to selectively add back Polymer 1.x's `undefined` rules | ||
* governing when observers & computing functions run based | ||
* on all arguments being defined (reference https://www.polymer-project.org/1.0/docs/devguide/observers#multi-property-observers). | ||
* | ||
* When loaded, all legacy elements (defined with `Polymer({...})`) | ||
* will have the mixin applied. The mixin only restores legacy data handling | ||
* if `_legacyUndefinedCheck: true` is set on the element's prototype. | ||
* | ||
* This mixin is intended for use to help migration from Polymer 1.x to | ||
* 2.x+ by allowing legacy code to work while identifying observers and | ||
* computing functions that need undefined checks to work without | ||
* the mixin in Polymer 2. | ||
* | ||
* @mixinFunction | ||
* @polymer | ||
* @summary Mixin to selectively add back Polymer 1.x's `undefined` rules | ||
* governing when observers & computing functions run. | ||
*/ | ||
Polymer.LegacyDataMixin = Polymer.dedupingMixin(superClass => { | ||
|
||
/** | ||
* @polymer | ||
* @mixinClass | ||
* @implements {Polymer_LegacyDataMixin} | ||
*/ | ||
class LegacyDataMixin extends superClass { | ||
/** | ||
* Overrides `Polyer.PropertyEffects` to add `undefined` argument | ||
* checking to match Polymer 1.x style rules | ||
* | ||
* @param {!Array<!MethodArg>} args Array of argument metadata | ||
* @param {string} path Property/path name that triggered the method effect | ||
* @param {Object} props Bag of current property changes | ||
* @return {Array<*>} Array of argument values | ||
* @private | ||
*/ | ||
_marshalArgs(args, path, props) { | ||
const vals = super._marshalArgs(args, path, props); | ||
// Per legacy data rules, single-property observers (whether in `properties` | ||
// and in `observers`) are called regardless of whether their argument is | ||
// undefined or not. Multi-property observers must have all arguments defined | ||
if (this._legacyUndefinedCheck && vals.length > 1) { | ||
for (let i=0; i<vals.length; i++) { | ||
if (vals[i] === undefined) { | ||
// Break out of effect's control flow; will be caught in | ||
// wrapped property effect function below | ||
const name = args[i].name; | ||
throw new UndefinedArgumentError(`Argument '${name}' is undefined. Ensure it has an undefined check.`, name); | ||
} | ||
} | ||
} | ||
return vals; | ||
} | ||
|
||
/** | ||
* Overrides `Polyer.PropertyEffects` to wrap effect functions to | ||
* catch `UndefinedArgumentError`s and warn. | ||
* | ||
* @param {string} property Property that should trigger the effect | ||
* @param {string} type Effect type, from this.PROPERTY_EFFECT_TYPES | ||
* @param {Object=} effect Effect metadata object | ||
* @return {void} | ||
* @protected | ||
*/ | ||
_addPropertyEffect(property, type, effect) { | ||
return super._addPropertyEffect(property, type, | ||
wrapEffect(effect, effect && effect.info && effect.info.methodName)); | ||
} | ||
|
||
/** | ||
* Overrides `Polyer.PropertyEffects` to wrap effect functions to | ||
* catch `UndefinedArgumentError`s and warn. | ||
* | ||
* @param {Object} templateInfo Template metadata to add effect to | ||
* @param {string} prop Property that should trigger the effect | ||
* @param {Object=} effect Effect metadata object | ||
* @return {void} | ||
* @protected | ||
*/ | ||
static _addTemplatePropertyEffect(templateInfo, prop, effect) { | ||
return super._addTemplatePropertyEffect(templateInfo, prop, wrapEffect(effect)); | ||
} | ||
|
||
} | ||
|
||
return LegacyDataMixin; | ||
|
||
}); | ||
|
||
// LegacyDataMixin is applied to base class _before_ metaprogramming, to | ||
// ensure override of _addPropertyEffect et.al. are used by metaprogramming | ||
// performed in _finalizeClass | ||
const Class = Polymer.Class; | ||
Polymer.Class = (info, mixin) => Class(info, | ||
superClass => mixin ? | ||
mixin(Polymer.LegacyDataMixin(superClass)) : | ||
Polymer.LegacyDataMixin(superClass) | ||
); | ||
|
||
// Apply LegacyDataMixin to Templatizer instances as well, and defer | ||
// runtime switch to the root's host (_methodHost) | ||
Polymer.Templatize.mixin = | ||
Polymer.dedupingMixin(superClass => class extends Polymer.LegacyDataMixin(superClass) { | ||
get _legacyUndefinedCheck() { | ||
return this._methodHost && this._methodHost._legacyUndefinedCheck; | ||
} | ||
}); | ||
|
||
console.info('LegacyDataMixin will be applied to all legacy elements.\n' + | ||
'Set `_legacyUndefinedCheck: true` to enable.'); | ||
|
||
})(); | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -814,7 +814,7 @@ | |
let context = inst._methodHost || inst; | ||
let fn = context[info.methodName]; | ||
if (fn) { | ||
let args = marshalArgs(inst.__data, info.args, property, props); | ||
let args = inst._marshalArgs(info.args, property, props); | ||
return fn.apply(context, args); | ||
} else if (!info.dynamicFn) { | ||
console.warn('method `' + info.methodName + '` not defined'); | ||
|
@@ -970,56 +970,6 @@ | |
return a; | ||
} | ||
|
||
/** | ||
* Gather the argument values for a method specified in the provided array | ||
* of argument metadata. | ||
* | ||
* The `path` and `value` arguments are used to fill in wildcard descriptor | ||
* when the method is being called as a result of a path notification. | ||
* | ||
* @param {Object} data Instance data storage object to read properties from | ||
* @param {!Array<!MethodArg>} args Array of argument metadata | ||
* @param {string} path Property/path name that triggered the method effect | ||
* @param {Object} props Bag of current property changes | ||
* @return {Array<*>} Array of argument values | ||
* @private | ||
*/ | ||
function marshalArgs(data, args, path, props) { | ||
let values = []; | ||
for (let i=0, l=args.length; i<l; i++) { | ||
let arg = args[i]; | ||
let name = arg.name; | ||
let v; | ||
if (arg.literal) { | ||
v = arg.value; | ||
} else { | ||
if (arg.structured) { | ||
v = Polymer.Path.get(data, name); | ||
// when data is not stored e.g. `splices` | ||
if (v === undefined) { | ||
v = props[name]; | ||
} | ||
} else { | ||
v = data[name]; | ||
} | ||
} | ||
if (arg.wildcard) { | ||
// Only send the actual path changed info if the change that | ||
// caused the observer to run matched the wildcard | ||
let baseChanged = (name.indexOf(path + '.') === 0); | ||
let matches = (path.indexOf(name) === 0 && !baseChanged); | ||
values[i] = { | ||
path: matches ? path : name, | ||
value: matches ? props[path] : v, | ||
base: v | ||
}; | ||
} else { | ||
values[i] = v; | ||
} | ||
} | ||
return values; | ||
} | ||
|
||
// data api | ||
|
||
/** | ||
|
@@ -2181,6 +2131,56 @@ | |
createMethodEffect(this, sig, TYPES.COMPUTE, runComputedEffect, property, dynamicFn); | ||
} | ||
|
||
/** | ||
* Gather the argument values for a method specified in the provided array | ||
* of argument metadata. | ||
* | ||
* The `path` and `value` arguments are used to fill in wildcard descriptor | ||
* when the method is being called as a result of a path notification. | ||
* | ||
* @param {!Array<!MethodArg>} args Array of argument metadata | ||
* @param {string} path Property/path name that triggered the method effect | ||
* @param {Object} props Bag of current property changes | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is a bag? perhaps restate as a "hash" or key value collection of ... |
||
* @return {Array<*>} Array of argument values | ||
* @private | ||
*/ | ||
_marshalArgs(args, path, props) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function has a non-trivial cyclical complexity score, can you please add unit tests to ensure the functionality is correct. Also it might be a sign that the logic should be split up into smaller more meaningful functions to maintain a higher degree of readability. |
||
const data = this.__data; | ||
let values = []; | ||
for (let i=0, l=args.length; i<l; i++) { | ||
let arg = args[i]; | ||
let name = arg.name; | ||
let v; | ||
if (arg.literal) { | ||
v = arg.value; | ||
} else { | ||
if (arg.structured) { | ||
v = Polymer.Path.get(data, name); | ||
// when data is not stored e.g. `splices` | ||
if (v === undefined) { | ||
v = props[name]; | ||
} | ||
} else { | ||
v = data[name]; | ||
} | ||
} | ||
if (arg.wildcard) { | ||
// Only send the actual path changed info if the change that | ||
// caused the observer to run matched the wildcard | ||
let baseChanged = (name.indexOf(path + '.') === 0); | ||
let matches = (path.indexOf(name) === 0 && !baseChanged); | ||
values[i] = { | ||
path: matches ? path : name, | ||
value: matches ? props[path] : v, | ||
base: v | ||
}; | ||
} else { | ||
values[i] = v; | ||
} | ||
} | ||
return values; | ||
} | ||
|
||
// -- static class methods ------------ | ||
|
||
/** | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason why we are _ prefixing this prototype value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a convention, we generally underscore any property not intended as part of the DOM element's public API, and consider them "protected". https://www.polymer-project.org/2.0/docs/devguide/properties#private-and-protected-properties