Skip to content

[3.x] Add legacy-data-mixin as 1.x->2.x/3.x migration aide. Fixes #5262. #5339

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

Merged
merged 7 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions lib/legacy/class.js
Original file line number Diff line number Diff line change
Expand Up @@ -334,16 +334,20 @@ function GenerateClassFromInfo(info, Base) {
*
* @param {!PolymerInit} info Object containing Polymer metadata and functions
* to become class methods.
* @template T
* @param {function(T):T} mixin Optional mixin to apply to legacy base class
* @return {function(new:HTMLElement)} Generated class
*/
export const Class = function(info) {
export const Class = function(info, mixin) {
if (!info) {
console.warn(`Polymer's Class function requires \`info\` argument`);
}
let klass = GenerateClassFromInfo(info, info.behaviors ?
const baseWithBehaviors = info.behaviors ?
// note: mixinBehaviors ensures `LegacyElementMixin`.
mixinBehaviors(info.behaviors, HTMLElement) :
LegacyElementMixin(HTMLElement));
LegacyElementMixin(HTMLElement);
const baseWithMixin = mixin ? mixin(baseWithBehaviors) : baseWithBehaviors;
const klass = GenerateClassFromInfo(info, baseWithMixin);
// decorate klass with registration info
klass.is = info.is;
return klass;
Expand Down
150 changes: 150 additions & 0 deletions lib/legacy/legacy-data-mixin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
/**
@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
*/

import { Class } from './class.js';
import { Polymer } from '../../polymer-legacy.js';
import { dedupingMixin } from '../utils/mixin.js';

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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surprised this is needed. Verify?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The version of babel we use in CLI doesn't handle subclassing builtins correctly; the latest one does, so will add TODO to remove once CLI updates to latest babel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: should be fixed by Polymer/tools#670

// 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.
*/
export const LegacyDataMixin = dedupingMixin(superClass => {

/**
* @constructor
* @extends {superClass}
* @unrestricted
* @private */
class LegacyDataMixin extends superClass {
/**
* Overrides `Polymer.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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider altering this method to get the info needed to present a good error? I'm concerned that since the intension is to warn, but the code throws here, we're depending on catching that where it's called.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, disregard that. It's clear that we need to abort the effect of marshaling the arguments and throwing let's us do this.

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;

});

Polymer.Class = (info, mixin) => Class(info,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just put LegacyDataMixin on top of the user class?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, the answer is no because the mixin customizes how properties are created on the element prototype. Since a subclass only creates properties that it defines, it will not modify properties created by its superclass.

superClass => mixin ?
mixin(LegacyDataMixin(superClass)) :
LegacyDataMixin(superClass)
);

console.info('LegacyDataMixin will be applied to all legacy elements.\n' +
'Set `_legacyUndefinedCheck: true` on element class to enable.');
Loading