freactive icon indicating copy to clipboard operation
freactive copied to clipboard

appendChild won't work with Polymer 1.0 "shady DOM"

Open sparkofreason opened this issue 10 years ago • 37 comments

See this discussion.

Bottom line: for performance reasons, Polymer 1.0 gives the option of not polyfilling the full shadow DOM spec by overwriting DOM API's like appendChild. For at least some Polymer elements, we need to execute slightly different code to ensure that things render correctly, allowing the element to be notified when its content has changed. I played with this a bit in my local copy of freactive, and was able to make it work by adding a register-append-child function in freactive.dom:

(def ^:private append-child-fns
  (atom {:default (fn [parent node] (.appendChild parent node))}))

(defn register-append-child
  [tag append-child-fn]
  (swap! append-child-fns assoc (.toUpperCase (name tag)) append-child-fn))

(defn- do-append-or-insert-child [parent node before]
  (if before
    (.insertBefore parent node before)
    (if-let [append-child-fn (@append-child-fns (.-tagName parent))]
      (append-child-fn parent node)
      ((@append-child-fns :default) parent node))))

And then calling it like this:

(dom/register-append-child :paper-button 
                           (fn [parent node] 
                             (.appendChild (js/Polymer.dom parent) node)))

The design maybe needs some more thought. Seems onerous to have to register for every element, maybe makes more sense to provide a predicate function. Probably could streamline performance a bit as well. I assume insertBefore has the same issue, but haven't tested.

sparkofreason avatar Jun 03 '15 16:06 sparkofreason

Okay, well it seems like a matter of changing all calls to DOM fn's to use Polymer.dom(parent) instead right? We just need to provide a wrapper method for all native DOM calls and then a plugin fn enable-polymer-dom! that replaces them with Polymer's version. Would that work?

aaronc avatar Jun 03 '15 16:06 aaronc

Yes, that sounds right.

sparkofreason avatar Jun 03 '15 16:06 sparkofreason

So, in general I've tried to design this system to be as pluggable as possible. If all native DOM calls need to be replaced, we just isolate them by default and then "patch" them if needed. Also, check out the new DOM implementation a little - everything is now based on IVirtualElement. If some element needs to do some wildly custom stuff it can do it as long as other elements see it as IVirtualElement. as-velem provides the syntatic sugar by converting all "hiccup vectors" to DOMElement and text to DOMTextNode - but maybe Polymer stuff would benefit from other conversions. The core of the library is really ReactiveElement and ReactiveElementCollection in freactive.ui-common plus ReactiveAttribute in freactive.core - none of which care at all which renderer they're using. Just some food for thought...

aaronc avatar Jun 03 '15 17:06 aaronc

A potential issue with patching DOM calls might occur if you were using more than one Web Components-ish library. I don't know that anyone else is doing something like Polymer shady-DOM right now, but it seems likely to pop up, and then we'd need a way to dispatch based on the library used to create the element. Does IVirtualElement solve this? Could we perhaps render the correct IVirtualElement implementation based on a tag namespace or prefix?

sparkofreason avatar Jun 03 '15 17:06 sparkofreason

Yes, see: https://github.com/aaronc/freactive/blob/develop/src/freactive/dom.cljs#L506-L513. register-node-prefix! will add a handler fn for a ns prefix: https://github.com/aaronc/freactive/blob/develop/src/freactive/dom.cljs#L95-L96 .

On Wed, Jun 3, 2015 at 1:28 PM, Dave Dixon [email protected] wrote:

A potential issue with patching DOM calls might occur if you were using more than one Web Components-ish library. I don't know that anyone else is doing something like Polymer shady-DOM right now, but it seems likely to pop up, and then we'd need a way to dispatch based on the library used to create the element. Does IVirtualElement solve this? Could we perhaps render the correct IVirtualElement implementation based on a tag namespace or prefix?

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-108536203.

aaronc avatar Jun 03 '15 17:06 aaronc

Do you see any need to dispatch on non-prefixed tags? dom-element could have custom dispatch potentially... - https://github.com/aaronc/freactive/blob/develop/src/freactive/dom.cljs#L486

On Wed, Jun 3, 2015 at 1:31 PM, Aaron Craelius [email protected] wrote:

Yes, see: https://github.com/aaronc/freactive/blob/develop/src/freactive/dom.cljs#L506-L513. register-node-prefix! will add a handler fn for a ns prefix: https://github.com/aaronc/freactive/blob/develop/src/freactive/dom.cljs#L95-L96 .

On Wed, Jun 3, 2015 at 1:28 PM, Dave Dixon [email protected] wrote:

A potential issue with patching DOM calls might occur if you were using more than one Web Components-ish library. I don't know that anyone else is doing something like Polymer shady-DOM right now, but it seems likely to pop up, and then we'd need a way to dispatch based on the library used to create the element. Does IVirtualElement solve this? Could we perhaps render the correct IVirtualElement implementation based on a tag namespace or prefix?

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-108536203.

aaronc avatar Jun 03 '15 17:06 aaronc

Let me play with it. My first impression is that register-node-prefix! and IVirtualElement covers it.

sparkofreason avatar Jun 03 '15 18:06 sparkofreason

Okay. Probably because a custom element can use it's own "as-velem" for children that may cover it.

On Wed, Jun 3, 2015 at 2:16 PM, Dave Dixon [email protected] wrote:

Let me play with it. My first impression is that register-node-prefix! and IVirtualElement covers it.

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-108554365.

aaronc avatar Jun 03 '15 18:06 aaronc

Looking at the implementation of DOMElement as a starting point. It seems that PolymerDOMElement would share most of that implementation. Could we instead make the DOM API an argument to DOMElement? Or maybe that's what you meant by patching DOM calls.

sparkofreason avatar Jun 03 '15 18:06 sparkofreason

Yeah, we could take either approach. What do you think is better? I don't know enough about Polymer/custom element internals personally.

FYI, added some doc strings here: https://github.com/aaronc/freactive/blob/develop/src/freactive/ui_common.cljs#L5-L34

On Wed, Jun 3, 2015 at 2:30 PM, Dave Dixon [email protected] wrote:

Looking at the implementation of DOMElement as a starting point. It seems that PolymerDOMElement would share most of that implementation. Could we instead make the DOM API an argument to DOMElement? Or maybe that's what you meant by patching DOM calls.

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-108564063.

aaronc avatar Jun 03 '15 18:06 aaronc

Due to other issues that I am currently working through, I will probably decide to pass a "native dom wrapper" through the whole tree so this may provide a way to deal with this.

aaronc avatar Jun 03 '15 19:06 aaronc

From what I can tell from their docs and code, that seems like it will be the key piece for Polymer 1.0.

sparkofreason avatar Jun 03 '15 19:06 sparkofreason

So regarding Polymer's manipulation of DOM elements. If freactive uses Polymer's shady DOM API, wouldn't the DOM appear unchanged to freactive? I think everything would work fine if this were the case. The only real issue I can see would potentially be with using freactive's element sequences - in that if other DOM elements are inserted, then the ordering of elements could maybe be messed up. But again, if shady DOM hides all this manipulation, there probably wouldn't be an issue...

aaronc avatar Jun 05 '15 21:06 aaronc

I guess the key question is whether freactive is going to expect things to maintain a specific place in the DOM, i.e. in the <paper-button> example, are you going to care that the <span> is not actually the immediate child of <paper-button> in the final version of the DOM? If you're always holding references to DOM elements and working directly with them (which I think has been the case so far), it seems like it should be fine. Same for lists, under the assumption that you don't care that maybe the whole list of elements has been moved in the DOM, as long as the items are kept in the same order.

When I get there with Cereus, I'll use the freactive interfaces for moving nodes, and that should keep things consistent.

sparkofreason avatar Jun 05 '15 21:06 sparkofreason

This implementation always holds its own references to nodes and keeps track of where it has placed nodes in the DOM. It would care if a list of children were moved if the reactive collection binding is being used because now things won't get inserted into the right places. But my understanding of Polymer's DOM API is that it would make it appear that things are the same place freactive put them, right? If that's the case then it wouldn't care what's going on in the final DOM.

aaronc avatar Jun 05 '15 21:06 aaronc

Agreed.

sparkofreason avatar Jun 05 '15 21:06 sparkofreason

The implementation for moving around nodes is here: https://github.com/aaronc/freactive/blob/develop/src/freactive/ui_common.cljs#L210-L234. This stuff is still sort of a work in progress, but the main idea is that this API would be used for managing movement: https://github.com/aaronc/freactive.core/blob/master/src/clojure/freactive/core.cljc#L877-L912 and would more or less expect that one projection source is managing a single projection target. Yes, it would be possible to move nodes between projection sources (for say drag and drop) if they knew how to do that. ReactiveElementCollections can be nested - no need for wrapper nodes.

aaronc avatar Jun 05 '15 21:06 aaronc

Looks good. Presumably, when I want to do something similar as Polymer, moving things as rendered in "logical" DOM to another place in the actual DOM, I would implement IVirtualElement to ensure that the parent gets notified when the logical child DOM changes.

BTW, I verified freactive works find doing mount!/unmount! on the native shadow root. So you're future-proof ;-)

sparkofreason avatar Jun 06 '15 00:06 sparkofreason

So I think I have a relevant case for projection. The Polymer <paper-dialog> element uses z-order to ensure that the dialog appears on top of everything else. However, if <paper-dialog> is a child of another element with z-order set then it doesn't work. Their recommendation is to make the <paper-dialog> a child of <body> if possible. I could make a custom element to do this directly via DOM API, but from what you described it sounds like I could rig it so I could put <paper-dialog> in a natural location in the logical DOM, but have it render as a child of <body>. Is that correct?

sparkofreason avatar Jun 11 '15 00:06 sparkofreason

You could make your own custom virtual element that inserts it where you like. Projection is more for binding collections. On Wed, Jun 10, 2015 at 8:40 PM Dave Dixon [email protected] wrote:

So I think I have a relevant case for projection. The Polymer element uses z-order to ensure that the dialog appears on top of everything else. However, if is a child of another element with z-order set then it doesn't work. Their recommendation is to make the a child of if possible. I could make a custom element to do this directly via DOM API, but from what you described it sounds like I could rig it so I could put in a natural location in the logical DOM, but have it render as a child of . Is that correct?

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-110955658.

aaronc avatar Jun 11 '15 01:06 aaronc

I guess this case is actually covered if we have the ability to specify the DOM API implementation.

Maybe a nice-to-have for this would be the ability to register the DOM API implementation by tag name.

sparkofreason avatar Jun 11 '15 13:06 sparkofreason

Well, I'm not sure it would be covered by the DOM API - does Polymer take care of appending it to the body? If not, an IVirtualElement could do that. Once we abstract the DOM API, we can figure out different ways to swap it in and out. I'm thinking that for Polymer we could just have a mount-polymer! fn which causes all child elements to use its DOM API. Would that work?

On Thu, Jun 11, 2015 at 9:48 AM, Dave Dixon [email protected] wrote:

I guess this case is actually covered if we have the ability to specify the DOM API implementation.

Maybe a nice-to-have for this would be the ability to register the DOM API implementation by tag name.

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-111141625.

aaronc avatar Jun 11 '15 14:06 aaronc

I was going to provide my own implementation of the DOM API which did the magic for the insert/append cases, otherwise called through to Polymer's DOM API. But thinking about it, the semantics might get sticky on how you associate a particular DOM API implementation with a particular tag, e.g. does it only apply to operations involving the element itself, or the element and it's children? So IVirtualElement is probably clearer.

Would mount-polymer! mean that the Polymer API's are used on all descendants? I'm guessing some other component frameworks play similar API tricks - Cereus certainly will, I think document-register-element does (at least for IE8 polyfills), maybe famo.us, etc. Would virtual elements provide more mix/match flexibility, for example a Polymer element could have Cereus elements as children?

For the Polymer case, I was thinking register-node-prefix! for the polymer namespace, and having the handler figure out which IVirtualElement impl to use based on the tag name. Similarly, Cereus could provide it's own namespace/handler/virtual elements, etc.

sparkofreason avatar Jun 11 '15 16:06 sparkofreason

Yeah, there are many different ways it could be done. One way is to have DOMElement, DOMTextNode, etc. take a DOM API instance as an argument. Each DOM API could have its own as-velem fn. Or you can create your own DOMElement, etc. implementations that use Polymer's API without changing the way DOMElement, etc. are now and a special as-velem fn and mount fn for polymer... Or you can use node prefix plugins. Once you're within a plugin, it gets to decide how all its children are handled - so you could have a [:polymer/polymer-root ...] node that inside of that uses polymer's virtual elements and DOM API and as-velem fn... It's flexible. I just tried to make ReactiveAttribute, ReactiveElement and ReactiveElementSequence as generic as possible... I could make DOMElement more generic too or maybe not... The main thing to keep in mind is that it's as-velem that decides how a given syntax (i.e. hiccup vector, etc.) gets converted to a virtual element - you can have different as-velem implementations and all the types in ui-common take it as a parameter (velem-fn).

On Thu, Jun 11, 2015 at 12:41 PM, Dave Dixon [email protected] wrote:

I was going to provide my own implementation of the DOM API which did the magic for the insert/append cases, otherwise called through to Polymer's DOM API. But thinking about it, the semantics might get sticky on how you associate a particular DOM API implementation with a particular tag, e.g. does it only apply to operations involving the element itself, or the element and it's children? So IVirtualElement is probably clearer.

Would mount-polymer! mean that the Polymer API's are used on all descendants? I'm guessing some other component frameworks play similar API tricks - Cereus certainly will, I think document-register-element https://github.com/WebReflection/document-register-element does (at least for IE8 polyfills), maybe famo.us, etc. Would virtual elements provide more mix/match flexibility, for example a Polymer element could have Cereus elements as children?

For the Polymer case, I was thinking register-node-prefix! for the polymer namespace, and having the handler figure out which IVirtualElement impl to use based on the tag name. Similarly, Cereus could provide it's own namespace/handler/virtual elements, etc.

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-111201639.

aaronc avatar Jun 11 '15 17:06 aaronc

Ok, I think I'm starting to get it. Seems like implementing IVirtualElement is for the case where you want to completely take over the DOM management from that point forward, maybe if you were using something like React Material UI.

The case of HTML custom elements is more light weight. A Polymer element needs to push in the DOM API, but otherwise could be handled by the as-velem, as it's really just an HTMLElement, and its children should be processed the same as any other "normal" DOM. Cereus is covered in a similar manner. Could we have as-velem take the DOM API as an argument, and the tag-handler for a namespace just pass as-velem? Seems pretty simple then. An element like [:polymer/paper-button] is processed by the polymer tag handler, strips the namespace from the tag, and calls as-velem with the updated element spec and the appropriate DOM API.

sparkofreason avatar Jun 11 '15 18:06 sparkofreason

Yeah implementing IVirtualElement is for when you want to basically make "anything" look like a logical child - could be one real element, several real elements, an element somewhere else - you could even make your own shadow DOM if you wanted.

We could have:

(defn as-velem* [native-api]
 (fn as-velem [x]
   ...))

(def as-velem (velem* native-dom))
(def as-velem-polymer (velem* polymer-dom))

aaronc avatar Jun 11 '15 18:06 aaronc

With this approach all child elements would use the same as-velem unless some plugin handler changed that...

aaronc avatar Jun 11 '15 18:06 aaronc

That should do it.

sparkofreason avatar Jun 11 '15 20:06 sparkofreason

I'm at a good spot in my project where I could tackle this, if you want some help.

sparkofreason avatar Jun 18 '15 15:06 sparkofreason

That would be great. I will try to send a few notes later today on a suggested DOM API protocol. On Thu, Jun 18, 2015 at 11:37 AM Dave Dixon [email protected] wrote:

I'm at a good spot in my project where I could tackle this, if you want some help.

— Reply to this email directly or view it on GitHub https://github.com/aaronc/freactive/issues/45#issuecomment-113195612.

aaronc avatar Jun 18 '15 16:06 aaronc