Re-added support for attaching events to document fragments

[Wildhoney]

I thought I'd jump straight in and make the change 👍

In the open issue it was mentioned that @spicyj wrote this part of the code, so it would be awesome if he — or anybody else — could chime in about the optimal way of implementing this. Thanks!

[jimfb]

@Wildhoney What will happen if you mount into a div that lives in a document fragment? Will this do the right thing, or do we need to walk up toward the top to discover the fragment that we live within?

[Wildhoney]

As far as I can see, the containerInfo._node in this instance will be the container we rendered to – but let's see what @spicyj or others think. There may be a perfectly reasonable explanation as to why React 15.x doesn't support rendering into a #document-fragment, rather than an oversight.

[jimfb]

I talked with @spicyj on internal chat, he just didn't know it was a use case. I don't think there is any reason not to support this.

My question though was with regard to ensuring that the document fragment gets selected as the document if we are mounting into a div within that document fragment.

var proto = Object.create(HTMLElement.prototype, {
  createdCallback: {
    value: function() {
      var mountPoint = document.createElement('div');
      this.createShadowRoot().appendChild(mountPoint);

      var name = this.getAttribute('name');
      var url = 'https://www.google.com/search?q=' + encodeURIComponent(name);
      ReactDOM.render(<a href={url}>{name}</a>, mountPoint);
    }
  }
});
document.registerElement('x-search', {prototype: proto});

In this case, we are mounting into the div, so nodeType === DOC_FRAGMENT_TYPE; will return false, which makes me wonder if doc will be pointing to the right place. I assume the "right place" would be the document fragment, but I just want to confirm that we get this right.

[jimfb]

Specifically, I'm concerned that mountPoint._ownerDocument will be undefined in that case, which will result in events not working correctly.

[Wildhoney]

Ah — you're right.

const shadowRoot = findDOMNode(this).attachShadow({ mode: 'open' });
const container = <span onClick={() => console.log('...')}>Node</span>;
const divNode = shadowRoot.appendChild(document.createElement('div'));
render(container, divNode);

Which doesn't handle the onClick event correctly in the modified 15.x version — however nor does it work with 0.14.x. I suppose a better long-term implementation is required, but at least we're fixing a regression — and although it's not a perfect solution — we haven't introduced a regression by making this change.

I've tried to err on the side of caution with mountPoint._node in that _node must be defined and be a type of DOC_FRAGMENT_TYPE for it to be considered a candidate for rendering inside of the container.

[jimfb]

Yeah, ok, I buy that. I'll accept to fix the regression. We should handle the other case at some point. Would you be willing/interested in doing a followup PR to handle that?

[Wildhoney]

Absolutely — if I can find a way to select #document-fragment nodes from a child node. Will investigate.

[jimfb]

@Wildhoney Ideally there would be a fast/easy way to get it directly. Worst case, it might require walking up the DOM nodes' parentNode until you find a document/fragment (or null). Obviously we'd only want to do that on initial mount and cache the value (maybe emit a warning if we can't find a good root).

[Wildhoney]

I'm slightly wary of the latter, as it seems to be an expensive task for what is possibly an edge-case.

Nevertheless, thanks for your help @jimfb — leave it with me 👍

[gurkerl83]

In React 0.14 there was an approach to enable event.path handling in ReactEventListener. This function was removed in React 15. Previously we had to switch the comment block in handleTopLevelImpl in order to enable event handling capabilities in shadow-dom.

Thanks,

[jhudson8]

Event bindings (like onClick for a button) don't work in React (15.4.2) within a shadow root. It seemed like that issue was going to be resolved from this convo - is there any progress happening to fix that issue that someone could like me to? Thanks