Skip to content
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

Templatizer: bug in templatize depending on the parent properties (and the browser) #3063

Closed
plequang opened this issue Nov 20, 2015 · 3 comments

Comments

@plequang
Copy link

Hi,

I'm using Polymer.Templatizer behavior in a custom element, and the templatize fails sometimes under some precise (and strange) circumstances.

I've been able to reproduce the issue in this jsbin https://jsbin.com/kivojomito/edit?html,console,output

To reproduce the issue, click first on one of the "Add content" button. Then click on the second button.

You should get the following error :

Uncaught TypeError: template._propertySetter is not a function
Polymer.Templatizer._extendTemplate @ polymer.html:3731
Polymer.Templatizer._prepParentProperties @ polymer.html:3700
Polymer.Templatizer.templatize @ polymer.html:3584

I get the error only on Chrome (v 46.0.2490.86), not FF.

I've been investigating a bit, and I've come to really strange findings.

I got the error only when the template uses a parent bound property named exactly _hasActions.
The error occurs only on the second instance of the templatizer custom element, i.e. on the second call to templatize.
You can avoid the error by adding a fake binding to the template.

I think the problem is in _extendTemplate :

    _extendTemplate: function(template, proto) {
      var n$ = Object.getOwnPropertyNames(proto);
      for (var i=0, n; (i<n$.length) && (n=n$[i]); i++) {
        var val = template[n];
        var pd = Object.getOwnPropertyDescriptor(proto, n);
        Object.defineProperty(template, n, pd);
        if (val !== undefined) {
          template._propertySetter(n, val);
        }
      }
    },

In this code, there is a potential issue because the _propertySetter function is part of the proto. So if in the array returned by Object.getOwnPropertyNames(proto);, the function "_propertySetter" is after a property with a value that is not undefined, the you can get the error above.

This is exactly what is happening in my example: the property _parent__hasAction appears before _propertySetter.

I've found that the parent properties are added to the template in _prepParentProperties, using Polymer.Bind._createAccessors which uses Object.defineProperty.
And the second time Object.defineProperty is called for _parent__hasAction, the order of the properties is completely changed.

I can understand that if these properties are stored using some hashtable data structure, the order cannot be guaranteed, but it is strange that it fails only the second time.

@kevinpschaaf
Copy link
Member

Confirmed, this is a bug. As you noted the hasOwnPropertyNames order can be completely different, so we need a guarantee that _propertySetter is installed first.

@kevinpschaaf
Copy link
Member

Will be fixed in #3408 (still need to add tests).

JSBin using the fix:
https://jsbin.com/xagehu/edit?html,console,output

Thanks for the awesome repro

@plequang
Copy link
Author

Cool. Thanks to you for the fix.

sorvell pushed a commit that referenced this issue Feb 13, 2016
Ensure _propertySetter is installed first. Fixes #3063
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants