Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

Fix IronMeta 1.x not working with iron-iconset-svg #71

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

btelles
Copy link

@btelles btelles commented Jun 8, 2017

I think the following line breaks backward compatibility with IronMeta 1.x:
https://github.com/PolymerElements/iron-iconset-svg/blob/master/iron-iconset-svg.html#L176

We should be forcing IronMeta 2.x and disallowing 1.x.

See bug #69
#69

I think the following line breaks backward compatibility with IronMeta 1.x:
https://github.com/PolymerElements/iron-iconset-svg/blob/master/iron-iconset-svg.html#L176

We should be forcing IronMeta 2.x and disallowing 1.x.

See bug PolymerElements#69
PolymerElements#69
@e111077
Copy link
Contributor

e111077 commented Jun 9, 2017

@bicknellr thoughts? Also @notwaldorf here's another issue with versioning :(

@notwaldorf
Copy link
Contributor

We can't pin this to only 2.0, because this would make this element not hybrid. I think the problem is that iron-meta is not hybrid, or that its breaking changes are not taken into consideration correctly (what i mean is: paper-input stamps different templates to avoid problems with iron-input. why isn't this element doing work to work with both iron-meta versions?)

@bicknellr
Copy link
Contributor

It looks like iron-meta 1.x doesn't really work in Polymer 2 because of two main reasons:

The combination of these two differences result in a situation where the Polymer.IronMeta constructor is safe to use in the same way both when you're running iron-meta 2.x on Polymer 2 and iron-meta 1.x on Polymer 1 but is not the same when running iron-meta 1.x on Polymer 2:

  • When running iron-meta 1.x on Polymer 1, Polymer.IronMeta constructs an element with its properties 'enabled' and the arguments to the constructor are passed to factoryImpl.

  • When running iron-meta 2.x on Polymer 2, Polymer.IronMeta constructs a regular object which has its own constructor definition to handle arguments and has plain getters and setters defined that work immediately.

  • When running iron-meta 1.x on Polymer 2, Polymer.IronMeta constructs an element which (a) doesn't receive the arguments to the constructor through factoryImpl and (b) doesn't have its properties 'enabled'.

This means that if you want to support both iron-meta 1.x and 2.x running in Polymer 2 you need to work around these differences with two particular changes:

  • You can't pass arguments when constructing the iron-meta as this isn't supported in iron-meta 2.x because Polymer 2 ignores them. Instead, you should set them manually after creating the element / object.

  • If you're using the Polymer element version of iron-meta (1.x, or document.createElement('iron-meta') in 2.x), you need to explicitly enable properties by either (a) attaching the element to the document and waiting until after connectedCallback would be called or (b) calling _enableProperties.

So, the intersection of the compatible API is:

  • Create the iron-meta with document.createElement('iron-meta') so that you always get an element.
  • Call _enableProperties if it exists (i.e. if your running in Polymer 2).
  • Then, after the above, set the type, key, value properties on the element.

@bicknellr
Copy link
Contributor

bicknellr commented Jun 14, 2017

I've opened PRs for iron-icon, iron-iconset, and iron-iconset-svg. IronValidatableBehavior and IronValidatorBehavior also look like they need to be updated if they're intended to work with both iron-meta 1.x and 2.x.

@bicknellr
Copy link
Contributor

Just added a PR for iron-icons.

@bicknellr
Copy link
Contributor

After mulling this over a bit, I'm not sure that what I've done in these PRs makes sense. If it's true that iron-meta 2 doesn't work in Polymer 1 with the same API, maybe we should change iron-meta's 2.x branch to indicate that it doesn't support both Polymer 1 and 2 but only 2? @cdata does that sound reasonable?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants