Skip to content

Conversation

sparkofreason
Copy link

We've been working with this fork and Polymer 1.0 for awhile now, and it seems pretty solid. Major differences between what's here and what was originally discussed:

  • Instead of native-bind-attrs!, INativeAPI just has native-get-attr and native-set-attr!. This was just to keep things simple, and because both the Polymer and native DOM implementations of attribute binding are exactly the same, outside of the attribute getter/setter.
  • Each element holds a reference to the relevant DOM API. In theory, the Polymer implementation would allow native HTML elements to also use the Polymer DOM, but that seems like a strong assumption. Each element also holds a reference to the parent DOM API, necessary to correctly look up the logical parent.

At some point it may be necessary to hook the .flush function for Polymer, but we haven't had any need for it yet.

The implementation of INativeAPI for Polymer can be found at https://github.com/allgress/cereus/blob/master/src/allgress/cereus/util.cljs

Sorry about the whitespace noise in project.clj. Need to practice my git-fu.

@aaronc
Copy link
Owner

aaronc commented Jul 9, 2015

Cool, so I may need a little bit of time to look through this thoroughly.

One thought that comes to mind is that I would prefer not to have to attach a native API to elements which are just using the default API. Also, I wonder if a lot of the complexity could be solved by using plain old object-oriented inheritance - a technique Clojure eschews a little too much I think. If we had a DOMElement class that had instance methods for doing all the native API stuff, you just create a PolymerDOMElement class that overrides the appropriate methods. Do you think this sort of approach would work? I feel uncomfortable with this technique of having to attach another manager object to each element - not just in this instance, but I have other code where Clojure forces me to attach what are effectively method overrides to instances. I think this stuff should/could be handled by inheritance.

@sparkofreason
Copy link
Author

I agree that inheritance is the right way to go, but I couldn't figure out how to override implementations in ClojureScript, since there's no extend. Do you know?

@aaronc
Copy link
Owner

aaronc commented Jul 9, 2015

Yeah, I've been looking into it a bit actually to solve the mess that
Cursor is in from needing to have four instance methods as fields - either
I rewrite the whole deftype for each variation or figure out some
relatively clean way to use inheritance.

On Thu, Jul 9, 2015 at 7:40 PM, Dave Dixon [email protected] wrote:

I agree that inheritance is the right way to go, but I couldn't figure out
how to override implementations in ClojureScript, since there's no extend.
Do you know?


Reply to this email directly or view it on GitHub
#53 (comment).

@aaronc
Copy link
Owner

aaronc commented Jul 10, 2015

I wrote a little macro that should handle the inheritance. I know this sort of thing is in generally frowned upon in the Clojure community, but honestly I think it gets really ugly when there's a genuine use case for it and the language doesn't allow it.

It works like this:

(deftype ParentType [a b c]
 Object
 (doSomething [this x])
 IProtocol1
 (-method1 [this]))

(defsubtype ChildType [a b c d]
 ParentType ;; use ParentType instead of Object
 (doSomething [this x]) ;; overrides parent instance method
 ;; automatically inherits native implementation of IProtocol1
 IProtocol2
 (-method2 [this]))

For this use case, I actually think we should put the DOM API as instance methods (no protocols) right on the wrappers - i.e.

(deftype DOMElement [...]
 Object
 (appendChild [this child] ...)
 (insertBefore ...)
 (setAttribute ...)
 ;; etc.
)

Then for Polymer we would have:

(defsubtype PolymerDOMElement [...]
 DOMElement
 (appendChild ...)
 ;; etc.

A custom as-velem fn would handle which tags get regular elements vs polymer elements.

I think this might let the core freactive.dom stay a little cleaner.

Your thoughts?

@aaronc
Copy link
Owner

aaronc commented Jul 10, 2015

Are you sure it's safe to use Polymer's DOM some places vs the native DOM other places? Can custom elements possibly mutate logical sub-trees in the shadow DOM? I'm just seeing that Polymer says "All DOM manipulation must use this API, as opposed to DOM API directly on nodes." here: https://www.polymer-project.org/1.0/docs/devguide/local-dom.html

@sparkofreason
Copy link
Author

Actually, I think there is a case where you conceivably need to use Polymer.dom in a subtree, and that's to support style scoping in shady DOM. They do some tricks where they rewrite class names to mimic shadow DOM CSS scoping. To support this you'd want to use Polymer.dom on the subtree. But then suppose someone is using components from another library with it's own custom DOM API, say cereus for the sake of discussion. How do we handle this case:

<complex-polymer-element>
  <div>Title<div>
  <complex-cereus-element>
    <div>...</div>
  </complex-cereus-element>
</complex-polymer-element>

Children of the cereus element need to use the cereus DOM API, so the Polymer API can't be pushed down the entire subtree. And I don't think simple inheritance works either, because of how the parent-node API has to reflect the API of the logical parent, and not the API used by the element (<complex-cereus-element> uses its own API to manipulate its subtree, but Polymer.dom to find its logical parent).

All if this seems like it's getting messy, but just caught this blurb in the Polymer docs which may show a way out:

Using these node mutation APIs when manipulating children ensures that shady DOM can distribute content elements dynamically. If you change attributes or classes that could affect distribution without using the Polymer.dom API, call distributeContent on the host element to force it to update its distribution.

Maybe easier just to supply a post-render hook instead of having to provide the entire API?

@aaronc
Copy link
Owner

aaronc commented Jul 10, 2015

Well the post-render hook would need to be applied to every operation including setAttribute I think. If the issue with inheritance is needing to find the parent element - this is already something that freactive does internally. The way it is designed now, freactive has two types of virtual elements - ones that wrap real native elements and ones that are purely virtual - elements need to walk up the tree to find a simple parent (one wrapping a native element). I think one could implement shadow DOM like capabilities with freactive alone as I've said before. Contrary to what the doc string says, -velem-parent returns the virtual parent element. native-parent is needed to walk up to the native parent. What we need maybe is to just walk up to the simple parent element and have the native dom methods directly on that native element wrapper. Would this maybe work?

@sparkofreason
Copy link
Author

I think I see what you're saying about parent, and I believe that will work. The virtual elements represent the logical DOM, and Polymer is free to move stuff actual DOM elements however it wants because we maintain the logical DOM relationships in the virtual elements. Is that correct? I'll make another fork and see how the subtyping approach shakes out.

On post-render: the only way it would be practical would be if all DOM mutations were batched and executed via RAF. I had gotten into my head that this was how things worked generally, but I see it's just for reactive elements. Any reason not to batch all DOM mutations?

@aaronc
Copy link
Owner

aaronc commented Jul 10, 2015

Yes, freactive maintains a tree of logical elements - but logical elements in freactive can either be native-backed - DOMElement, TextNode and UnmanagedDOMNode or virtual - ReactiveElement and ReactiveElementSequence. These virtual elements can actually be backed by 0 or more native-backed elements. So in freactive, there could be a logical child that freactive places somewhere else like polymer. What you'd need is to walk up to the logical native parent (which could be a virtual element to polymer). Does that make sense?

In order to get this working, you will need to change the mount behavior - which I was going to do anyway - so that mount! wraps an existing element in a managed DOMElement. Maybe I should just go ahead and do that.

All DOM mutations in freactive are batched via RAF except for the initial mount. Anywhere there is an element, sequence or attribute binding the updates get batched. Where else are you thinking of batching?

@sparkofreason
Copy link
Author

I was thinking of batching initial mount, with the idea of using distributeContent after render. Tried it, and a little surprisingly .distributeContent doesn't solve the problem.

What you describe around the logical parent makes sense. Moving on to the subtype approach.

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

Successfully merging this pull request may close these issues.

2 participants