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

Getters and setters made inaccessible #2348

Closed
nazar-pc opened this issue Aug 22, 2015 · 6 comments
Closed

Getters and setters made inaccessible #2348

nazar-pc opened this issue Aug 22, 2015 · 6 comments
Assignees

Comments

@nazar-pc
Copy link
Contributor

Polymer({
    'is'        : 'cs-nav-tabs',
    'extends'   : 'nav',
    properties  : {
        active  : {
            observer    : 'active_changed',
            type        : Number
        }
    },
    ready : function () {
        if (!this.querySelector('button[active]')) {
            this.active = 0;
        }
    },
    active_changed : function () {
        var buttons = this.querySelectorAll('button');
        var button;
        for (var index = 0; index < buttons.length; ++index) {
            button = buttons[index];
            button.active = index == this.active;
        }
    }
});
Polymer({
    'is'        : 'cs-button',
    'extends'   : 'button',
    properties  : {
        active  : {
            reflectToAttribute  : true,
            type                : Boolean
        }
    }
});
<nav is="cs-nav-tabs">
    <button is="cs-button" type="button">One</button>
    <button is="cs-button" type="button">Two</button>
</nav>

At the moment when button.active is assigned (at createdCallback of [is=cs-nav-tabs], you know), there are no getters/setters yet in button element, which eventually makes accessors below this newly declared property in prototypes chain and basically inaccessible.

Firefox Nightly, Shadow DOM polyfill

@nazar-pc
Copy link
Contributor Author

Forgot to write, under native Shadow DOM it works properly O_o.
Chromium 44.0.2403.89

@sjmiles
Copy link
Contributor

sjmiles commented Aug 25, 2015

Did you register cs-button before cs-nav-tabs? It's important to register dependencies of an element before the element itself.

@nazar-pc
Copy link
Contributor Author

Well, it is not actually a dependency, but yes, it turns out that cs-button is included earlier, I include all components in aphabet order.
Though since they do not extend each other order shouldn't matter I guess.
Code as is here, I've posted simplified example.

@sjmiles
Copy link
Contributor

sjmiles commented Aug 25, 2015

Because cs-nav-tabs consumes cs-button, cs-button is a dependency of cs-nav-tabs.

Where is the simplified example? Also, your code above is in CoffeeScript, which is not the lingua franca here, would be easier if it were in straight JavaScript.

@arthurevans
Copy link

If I'm reading this correctly, the cs-buttons are in the light DOM. However, cs-nav-tab is trying to interact with the children in its light DOM during the ready callback. Under polyfill, that's not working correctly because the children aren't upgraded.

See:
https://www.polymer-project.org/1.0/docs/devguide/registering-elements.html#ready-method

Specifically:

"Within a given tree, ready is generally called in document order, but you should not rely on the ordering of initialization callbacks between sibling elements, or between a host element and its light DOM children." (emphasis added)

There are a couple of workarounds here: instead of setting the property button.active (which requires the button be upgraded), you could set the attribute: button.setAttribute('active', foo) ... Slightly less efficient, but setting the attribute works even if the element hasn't been upgraded yet.

Or: have the button fire an event to the parent from its attached callback, to let it know it's upgraded and ready to talk.

There is an open feature request to add a callback when an element's distributed children change -- I think that will make a lot of these cases simpler.

@nazar-pc
Copy link
Contributor Author

Because cs-nav-tabs consumes cs-button, cs-button is a dependency of cs-nav-tabs.

I thought elements are upgraded when all of them are registered, so their order in DOM and order of importing doesn't actually matter unless they extend each other.

Where is the simplified example? Also, your code above is in CoffeeScript, which is not the lingua franca here, would be easier if it were in straight JavaScript.

Simplified example should be like this:
http://jsbin.com/rajahopudi/1/edit?html,output

But that particular example doesn't reproduce in isolation an issue I experience.
Also rewrote initial example in plain JS.

@arthurevans, actually it seems to be true, however, please, take a look at PR #2349, it actually checks whether anything among declared properties was assigned in between directly, uses that and drops original property to open access to getters/setters again. This doesn't hurt performance in any significant way, but solves this issue and one more very efficiently.

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

6 participants