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

Dynamically publishing attributes #1198

Closed
positlabs opened this issue Feb 17, 2015 · 16 comments
Closed

Dynamically publishing attributes #1198

positlabs opened this issue Feb 17, 2015 · 16 comments

Comments

@positlabs
Copy link

It doesn't seem like there's a built-in way to dynamically publish attributes in Polymer components. I think there should be.

I understand that publishing attributes during element registration is much more efficient. However, I'm writing a wrapper for a webgl lib where attributes are unknown until the elements are instantiated.

Here is an example of what I'm trying to achieve:

<seriously-graph linear>
    <seriously-source>
        <img src="images/pencils.jpg">
    </seriously-source>
    <seriously-effect type="pixelate" pixelSize="{{pixelSize}}"></seriously-effect>
    <seriously-effect type="blur" amount=".5"></seriously-effect>
    <seriously-target width="411" height="425"></seriously-target>
</seriously-graph>

Under the hood, this is what is happening in the effect nodes:

var effect = seriously.effect(this.type); // type is defined at runtime
var attributes = effect.inputs(); // these are the attributes this component instance cares about

Ideally I would just use a method from Polymer to dynamically publish attributes. Since I haven't found a way to do this, I have used MutationObserver to detect changes to attributes. This approach sort of works, but then I need to worry about serialization.

Here is the project repository:
https://github.com/positlabs/seriously.polymer

And some SO questions I've created:
http://stackoverflow.com/questions/28519217/dynamically-publish-attributes-in-polymer (check out that phat +50 bounty)
http://stackoverflow.com/questions/28553399/serialize-unpublished-polymer-attributes

@positlabs
Copy link
Author

My initial approach to this was to publish a generic props attribute that would accept an object.

<seriously-effect type="pixelate" props="{{ {pixelSize: pixelSize} }}"></seriously-effect>

It works, but it's not pretty. I want it to be pretty!!

@meandmycode
Copy link

Whilst I'm sure there is a way to accomplish this in the fashion you have, you have a sort of anti-pattern going on with the type attribute, custom elements themselves already distinguish isolation of behavior and functionality (like a type or class), have you considered instead something like:

<seriously-graph linear>
    <seriously-source>
        <img src="images/pencils.jpg">
    </seriously-source>
    <seriously-pixelate pixelSize="{{pixelSize}}"></seriously-pixelate>
    <seriously-blur amount=".5"></seriously-blur>
    <seriously-target width="411" height="425"></seriously-target>
</seriously-graph>

This would better support extension of your framework too if you can establish an API for which these elements adhere.

For example, I could develop:

<levels-effect black-point="100"></levels-effect>

That could be added to a graph element from your framework.

@positlabs
Copy link
Author

That would work, but SeriouslyJS has a lot of plugins, and the list is always growing. It will make maintenance difficult for me. I'm trying as much as possible to just forward props to the effect nodes (with the exception of type, which is used to create the node).

I thought about writing a generator that would spit out component code for each effect, but it would make maintenance only marginally easier.

I kind of got dynamic attributes to work, but I really feel like I'm rewriting polymer (and probably not as well). I'm using MutationObserver, property accessors, and a deserialize method. Now I have to figure out how to force the template to update when these values change...

https://github.com/positlabs/seriously.polymer/blob/master/tests/dynamic-attributes/dynamic-attributes.html

@pflannery
Copy link

I wonder if injectBoundHTML would help you?

@positlabs
Copy link
Author

Maybe, but the main point is that Polymer needs a way to dynamically publish attributes. I'd rather not hack around this missing feature.

@meandmycode
Copy link

I wouldn't say you are hacking around it, polymer is aiming to be a veneer on top of pure web, so using mutation observers is fine. There are a bunch of systems in polymer that handle change notification and attribute reflection, but I assume they are only analysed when the polymer element is registered (worth looking into though). I do know that that v0.8 appears to be more explicit about properties so the trend seems to be going to statically proving the properties of a type.

In terms of the maintenance issue and registering new elements for each, you may want to look into dynamically registering elements - this may even be by purely using document.registerElement directly.

Right now I'd say you are doing something that isn't entirely considered in polymer/web components, for the most part registerElement is expecting you to pass a prototype that represents the shape of an element up front, having an element being so dynamic was solved by specialised systems like data attributes (and its matching dataset API).

@positlabs
Copy link
Author

The reason I use Polymer is because it's web component sugar. If dynamic attributes were to exist, I expect it would be a feature of Polymer.

I am fairly new to Polymer, but the biggest use-case I see for it is to wrap existing JS libs. There are a ton of JS libs, and not many web components. The simplest (most performant, easiest to maintain) way to do this is to forward attributes to instances of objects. The simplest way to do that is to have dynamic attributes.

I will look into dynamic element registry, but again, I feel like I'm rewriting something that Polymer already does very well. How difficult would it be to apply the same methods to instances of components, as opposed to prototypes? Probably not difficult at all.


Anyway, if I continue with this hack (to prove a point), my next question is "how does Polymer decide to serialize attributes in template bindings?". [Object object] is useless to me. Is this behavior documented anywhere. Can I implement the required state so Polymer serializes objects?

@ghost
Copy link

ghost commented Feb 23, 2015

I can see both sides of the argument. I would agree Polymer's "world view" shouldn't change. But I also see Polymer potentially benefiting from simple "wrapper" elements that integrate other UI libraries with Polymer.

But didn't you guys know? Polymer can save the world!! ok, not really, but here is a Gist that I think may appeal to everyone. It declares a new <seriously-effect> element, and has a sample index.html. Just enough of seriously.effect() is mocked to show that everything works.

@positlabs You can use <seriously-effect type="pixelate" pixelSize="{{pxSize}}"/>, and it works just like you'd expect. While @meandmycode, what's really happening is an actual polymer-element for that specific effect type is being dynamically created if necessary, and the original <seriously-effect> is then replaced with a new bound instance of that new effect-specific polymer element (named seriously-effect-<<type>> where <<type>> is the value of the type attribute).

This is kinda nice because the source uses <seriously-effect type="pixelate">, but when you "inspect elements" in browser, you'll actually see <seriously-effect-pixelate> has replaced the generic one with all bindings bound and working, and at the bottom of your document.body, you'll see <seriously-effect-types> which contains all the dynamically created <polymer-element name="seriously-effect-<<type>>"> declarations.

Would appreciate any feedback on this, and another Polymer project: PolyComp

Thx!

@positlabs
Copy link
Author

Nice! This seems to work.

I'm still confused about why the attributeChanged handler doesn't work if attributes are published. To make sure changes aren't handled twice? This behavior is not intuitive.

Here is a broken test (and source) I made based on the general approach you took to creating the dynamic elements. I'm not sure why this is happening, but it seems like polymer is scraping for element declarations multiple times, and then borking when it finds a duplicate. Any idea what's happening there?

Here's a working test and the source, built on your example (check console).

Anyway, this has been immensely helpful. Thank you! If you want to post the answer to the SO question I made, I'll give you the bounty. Otherwise, I can post it for posterity-sake.

@ghost
Copy link

ghost commented Feb 25, 2015

I'm still confused about why the attributeChanged handler doesn't work if attributes are published. To make sure changes aren't handled twice? This behavior is not intuitive.

Technically, it's properties that can be published, rather than attributes. Regardless, attributeChanged is called when an attribute changes. Do you have a simple reproduction of what you're seeing?

Here is a broken test (and source) I made based on the general approach you took to creating the dynamic elements. I'm not sure why this is happening, but it seems like polymer is scraping for element declarations multiple times, and then borking when it finds a duplicate. Any idea what's happening there?

Not sure what you mean by "based on the general approach", as the broken test is not doing a couple of critical things necessary for bindings to work as you'd expect. Polymer is not like other UI frameworks, in that it does not "scrape" or walk the entire DOM looking for special elements. Instead, it relies on the browser's built-in parser calling the corresponding Customer Element callbacks. It's therefore not clear what you mean by "seems like polymer is scraping"..

I did update the Gist with some tweaks, corrections, comments, and aligned to your working test.

I did post an answer on SO with a link to the Gist.

@pflannery
Copy link

@positlabs One thing I noticed in the broken test link you provided is that you are defining default values for arrays and objects beneath the publish property. It's not recommended you do that, its best to define objects and arrays as null then set their values during the created callback

For property values that are objects or arrays, you should set the default value in the created callback instead. This ensures that a separate object is created for each instance of the element
Quote from the documentation here

Basically when defining a polymer element we are defining the prototype and to access the instance we can use this during prototype calls and polymer callbacks.

@positlabs
Copy link
Author

@phestermcs
In the broken test, I get this error when trying to register more than one dynamic element.
Uncaught NotSupportedError: Failed to execute 'registerElement' on 'Document': Registration failed for type 'seriously-effect-blur'. A type with that name is already registered.

After looking into why it was failing, I found that it was because I was appending innerHTML to document.head instead of creating a temp div, assigning innerHTML, and grabbing the polymer-element from there.

// BAD. Somehow causes element declaration to register twice
var tmplString = '<polymer-element name="'+this.name+'"></polymer-element>';
document.head.innerHTML += tmplString;
//document.body.innerHTML += tmplString; // also fails, and causes lifecycle callbacks to fire twice on the element generator

//GOOD! Making a temporary, off-dom div and pulling the element out works.
var tmpDiv = document.createElement('div');
var tmplString = '<polymer-element name="'+this.name+'"></polymer-element>';
tmpDiv.innerHTML = tmplString;
document.body.appendChild(tmpDiv.children[0]);

Do you think this is a bug in Polymer? I suppose I could make a test case with vanilla web components and compare results.

I gave you the SO bounty. Thanks again for your help!

@pflannery, good point. I plan to remove the publish object since it didn't do what I thought it would. I was trying to make it serialize objects by hinting at the type.

@ghost
Copy link

ghost commented Feb 25, 2015

Do you think this is a bug in Polymer? I suppose I could make a test case with vanilla web components and compare results.

Not a bug in Polymer. When you do innerHTML += you're essentially doing this:

var html = tmpDiv.innerHTML + '<polymer-element/>';
// html is completely reparsed here
tmpDiv.innerHTML = html;

You're asking the browser to completely reparse everything, so it will reparse any previous <polymer-element>, which will try and register the element againg. It's doing exactly what you asked it to do :)

I had another update to the Gist, making it a slightly simpler, and more "compliant" with public Polymer API (using templateInstance instead of templateCreator_).

Just curious what is it you're trying to do in your broken test that is not being done in the example?

@ghost
Copy link

ghost commented Feb 25, 2015

Oops. One last little tweak in Gist. Anyways, glad it worked for you.

@positlabs
Copy link
Author

Just curious what is it you're trying to do in your broken test that is not being done in the example?

Just trying to wrap my head around the issue by starting from scratch. Your example is very comprehensive.

Anyway, it doesn't sound like there's much support for implementing dynamic attribute publishing in Polymer. I suppose we can close this ticket. Thanks again, everyone!

@positlabs
Copy link
Author

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

No branches or pull requests

3 participants