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

Event Handler on React Component not invoked when React Component is rendered inside a Web Component #9242

Closed
nilshartmann opened this issue Mar 23, 2017 · 30 comments

Comments

@nilshartmann
Copy link

Do you want to request a feature or report a bug?
Bug

What is the current behavior?
A React Component with an Event Handler (for example onClick) is rendered inside a Web Component. When the Component is clicked the Event does not receive the React Component (specified callback is not invoked)

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).

You can reproduce it with Web Component example contained in the react repository (https://github.com/facebook/react/blob/master/examples/webcomponents/index.html): Replace the 'a' element with a button and add for example an onClick event handler.

You can find a modified version of the Web Component example (based on the 15.4.2 codebase - https://github.com/facebook/react/blob/v15.4.2/examples/webcomponents/index.html) here:
https://gist.github.com/nilshartmann/3a520920e5fc920bfde49e077ad3beab#file-index-html-L50

What is the expected behavior?
The event handler should be called.

For testing I have modified getEventTarget.js to return the target from the path property of the nativeEvent (instead of the "original" target from the nativeEvent). With this addition it works -
the Event Handler is called.

You can find the modified version also in the gist: https://gist.github.com/nilshartmann/3a520920e5fc920bfde49e077ad3beab#file-geteventtarget-js-L6

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
15.4.x and 16.x
I've tested in Chrome, Firefox and Safari. I don't know if it works in previous versions of React (don't think so)

@DeanBDean
Copy link

@nilshartmann Should path be composedPath() instead?

https://hayato.io/2016/shadowdomv1/#getting-event-path

By the way, this almost entirely solves an issue I was having where events were not being captured by a web component that encapsulated a ReactDOM.

@mgrdevport
Copy link

@DeanBDean You are right. We were using Chrome to investigate the event bubbling issue months ago, which gave us access to the event.path property. We are using the webcomponentsjs polyfill to make the shadow dom available in all browsers. Today the polyfill (shadyDom) provides us the composedPath() function, so we should maybe change it to the following:

// If encapsulated in a Web Component use the composed Event path
if(nativeEvent.composedPath && nativeEvent.composedPath()) {
    return nativeEvent.composedPath()[0];
}

@bthallion
Copy link

I'm having the same problem, is there a workaround for this currently?

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

We should confirm whether it still happens on master with native web components.
(16.0.0 WC support is borked but I fixed it on master.)

@larkintuckerllc
Copy link

First, I have also observed this first hand with React 16.0.0 (today). Also confirmed that a fellow opened up a related issue and wrote a working temporary fix, https://www.npmjs.com/package/react-shadow-dom-retarget-events .

While I do not see a use case for web components in my React applications, I am considering using React in building more complicated web components to be delivered to other platforms (I am thinking for WordPress bloggers; or similar).

@rytisbac
Copy link

Same problem here... Created component with react and want to include it to other sites - similar as iframe. The only way it can work is to retarget events. But I think it realy hurts performace

@robdodson
Copy link

Does React use event delegation to listen for clicks on React components? If so I imagine the shadow boundary would get in the way of that... Just want to make sure I understand the problem :)

@xastor
Copy link

xastor commented Nov 22, 2017

The react-shadow-dom-retarget-events module works, but has a few problems of its own. If I'm correct, it picks up the native events on the shadow dom element, and then directly calls the react component event handlers (onClick, etc.. ) using those native events, instead of synthetic events. I think that because of this, stopPropagation() does not work.

It would be nice to see the React team pick this issue up and provide a proper fix. Shadow dom is gaining momentum! :-)

I've been thinking and maybe it would make sense to attach the global React event handlers to the element returned by getRootNode() instead of to the document?

https://developer.mozilla.org/en-US/docs/Web/API/Node/getRootNode

@marionebl
Copy link

marionebl commented Feb 5, 2018

Is there interest in a fix based on the suggestions by @nilshartmann? I'd be happy to contribute the required changes and new tests if required.


Edit:
If I understand correctly @xastor's suggestion is closely related to #2043, which appears to be the more general approach to fix this.

Reading the lengthy discussion of #8117, which was the last implementation attempt for #2043, I guess that redesigning the event propagation system to support multiple roots is a sprawling/hard to implement change.

@opichals
Copy link

opichals commented May 29, 2019

Tested successfully using the event.composePath() in https://github.com/opichals/react/tree/add-event-target-composePath-support based on #9242 (comment)

@renegare
Copy link

Not sure if this is still an issue with anyone; I however found the following addition to your react app entry point will work:

class CustomElement extends HTMLElement {
  connectedCallback() {
    // setup
    const shadow = this.attachShadow({ mode: "open" });
    const root = document.createElement("div");
    shadow.appendChild(root);

    // Making the shadow appear like document 
    // so react events work as normal
    Object.defineProperty(root, "ownerDocument", { value: shadow });
    shadow.createElement = (...args) => document.createElement(...args);

    ReactDOM.render(<App />, root);
  }
}
customElements.define("custom-element", CustomElement);
// DONT FORGET to add <custom-element></custom-element> to your page

The rationale here is:

  • react event system does work normally
  • shadow dom re-targeting is the issue
  • custom re-re-targeting is complex; and does not reliably work with events that do not bubble - example focus and blur
  • instead of react binding to document it should bind to the shadow. Because as far as css and js events that is the true document root of a shadow dom
  • react binds event listeners to the ownerDocument property of the root element it renders into. It expects that to be document.
  • createElement appears to be the only method missing from the shadow element, which react needs

This works with the create-react-app setup.

Any issues one can foresee taking this approach?

@renegare
Copy link

renegare commented Sep 23, 2019

if my assumptions are correct; this issue is not a problem or an issue on the react side at least.

But what could be cool is for ReactDOM.render to have a third options arg? which allows you better customize all global props it depends on to function e.g document.

This looks interesting and similar though: #13713

@GeorgeTaveras1231
Copy link

GeorgeTaveras1231 commented Sep 23, 2019

@renegare Your approach seemed to work for me except that there were a couple of other APIs that react depended on that I needed to add to the shadow root:

  • createElementNS
  • createTextNode

I'm surprised I didnt have to add createDocumentFragment 🤔

Actually found that react doesn't use this internally -- I suppose this is still surprising

@renegare
Copy link

I think this beats maintaining a list of events. Maintain a list of document methods

... but as you can see be aware this is a hack!

@patrickpietens
Copy link

patrickpietens commented Oct 7, 2019

This is definitely the best solution so far. However, I still can't get createPortal() to work.

EDIT: It seems that https://github.com/davidtheclark/focus-trap-react messes things up. Removing it made createPortal() work as well.

@chrisparton1991
Copy link

chrisparton1991 commented Oct 17, 2019

Unfortunately customElements.define is null inside Chrome extension content scripts (possibly Firefox too, I haven't checked). See https://bugs.chromium.org/p/chromium/issues/detail?id=390807.

I'm trying to convert a Chrome extension to use React. The content script uses a shadow DOM to avoid styling conflicts from the host page.

At the moment it looks like I'll have to remove the shadow DOM and do my best to override any weird styling that I come across.

EDIT: I got this working by creating my own shadow root and applying the above workarounds directly:

const root = document.body.appendChild(this.doc.createElement('div'));
const shadow = root.attachShadow({ mode: 'closed' });
const container = shadow.appendChild(this.doc.createElement('div'));

Object.defineProperty(container, 'ownerDocument', { value: shadow });
shadow.createElement = (...args) => document.createElement(...args);
shadow.createElementNS = (...args) => document.createElementNS(...args);
shadow.createTextNode = (...args) => document.createTextNode(...args);

ReactDOM.render((
  <button onClick={() => alert('I work!')}>Click me</button>
), container);

Still early days, but it's looking promising so far.

@scythargon
Copy link

Can't make SyntheticEvent's of react-bootstrap package work when it's component is rendered inside shadow DOM...

@renegare
Copy link

Unfortunately customElements.define is null inside Chrome extension content scripts (possibly Firefox too, I haven't checked). See https://bugs.chromium.org/p/chromium/issues/detail?id=390807.

yeah sounds like web components are yet to be enabled within the context of an extension. But why do you need it there, out of interest? I believe an extension sandboxed so you can do as you please (kinda) without the need of a custom element.

@chrisparton1991
Copy link

@renegare I'm using a shadow root because I need to inject an overlay onto particular websites, and the parent website styles will often conflict with mine. I've even had one site mutate my DOM to wrap my input fields.

@JonyE
Copy link

JonyE commented Dec 17, 2019

@adrian-dobre
Copy link

adrian-dobre commented Mar 4, 2020

Later edit: I originally misstated that the problem is in React Portals, but that's not true, the problem is in rc-utils, a dependency of Ant Design System.

Original post
I found one issue with @chrisparton1991 / @renegare 's solution: If you use React Portals the reference to owner document is lost (because it uses document.createElement directly instead of ownerDocument, see: https://github.com/react-component/util/blob/85c296bbf06966c37963da4e8b7ce5dfba367b1a/src/PortalWrapper.js#L102). You could try to avoid Portals usage, unfortunately you could rely on some libs that use Portals (like ant design system - modal).

However, if the parent container (passed in with getContainer) is created inside the shadowRoot (and it should be, otherwise this is not a problem), relying on the fact that Portal Wrapper does something like parent.appendChild you can rewrite parent.appendChild to also add ownerDocument :)

changeOwnerDocumentToShadowRoot(element: HTMLElement, appContainer: ShadowRoot) {
     Object.defineProperty(element, 'ownerDocument', {value: appContainer});
 }

augmentAppendChildWithOwnerDocument(elem: HTMLElement, appContainer: ShadowRoot) {
        const origAppChild = elem.appendChild;
        const propDesc = Object.getOwnPropertyDescriptor(elem, 'appendChild');
        if (!propDesc || propDesc.writable) {
            Object.defineProperty(elem, 'appendChild', {
                value: function (child: HTMLElement) {
                    changeOwnerDocumentToShadowRoot(child, appContainer);
                    origAppChild?.call(elem, child);
                }
            });
        }
    }

augmentCreateElementWithOwnerDocument(appContainer: ShadowRoot, createFnName: keyof Document) {
        const originalCreateFn = document[createFnName] as Function;
        appContainer[createFnName] = (...args) => {
            const element = originalCreateFn.call(document, ...args);
            changeOwnerDocumentToShadowRoot(element, appContainer);
            augmentAppendChildWithOwnerDocument(element, appContainer);
            return element;
        };
    }

changeOwnerDocumentToShadowRoot(appRootNode as HTMLElement, appContainer);
augmentCreateElementWithOwnerDocument(appContainer, 'createElement');
augmentCreateElementWithOwnerDocument(appContainer, 'createElementNS');
augmentCreateElementWithOwnerDocument(appContainer, 'createTextNode');

Hopefully this will help somebody having the same problem. Sorry for the somewhat confusing code, I pulled it out from my solution without cleanup.

Maybe PortalWrapper could be updated from:

if (!this.container) {
      this.container = document.createElement('div');
      const parent = this.getParent();
      if (parent) {
        parent.appendChild(this.container);
      }
    }

to

if (!this.container) {
      const parent = this.getParent();
      if (parent) {
        this.container = parent.ownerDocument.createElement('div');
        parent.appendChild(this.container);
      } else {
          this.container = document.createElement('div');
      }
    }

and that would fix the issue

@nlrowe
Copy link

nlrowe commented Mar 25, 2020

I switched from react-shadow-dom-retarget-events to renegare fix and all was working well in modern browsers. Unfortunately, this seems to be causing issues with the webcomponents pollyfill in at least edge and IE. I believe it is altering this.ownerDocument on this line and it throws 'SCRIPT65535: Invalid calling object'.

Update: It is the root element that is throwing that error as we are obviously changing the ownerDocument on it to be the shadow object. The pollyfill code expects it to be the document, which is what it is pollyfilling. If you add the pollyfill function being referenced on line 181, all renders fine. However, when you fire an event such as blur, the page dies. Probably a lot of normal assumptions being made that this change violates.

poeschko added a commit to WolframResearch/wolfram-notebook-embedder that referenced this issue Mar 26, 2020
…oot node

while also making it appear like a normal document
as suggested in facebook/react#9242 (comment)

Also adjust dimensions of shadow container as necessary.
@stevematney
Copy link

We've built out a fairly small solution for this problem called react-html-element. It's extremely early in its development, so it probably doesn't cover every use case, since there are certainly some we don't know about.

We'll be dogfooding this in our internal projects, but would love anybody that wants to try it out to do so! Please log issues if you find them, as we have a vested interest in it being robust and useful!

aindlq added a commit to researchspace/researchspace that referenced this issue Apr 22, 2020
…ponents.

This is ugly hack to workaround issue with event propagation when react root is inside custom web component.
See facebook/react#9242

This issue caused strange behaviour of platform components inside semantic-map popovers and Mirador annotation popovers.

Basically even `semantic-link` was not working properly there and instead of changing the content of the page dynamically it reloaded the whole page.
Or for example it was not possible to use `mp-event-trigger` in such context.

Signed-off-by: Artem Kozlov <artem@rem.sh>
@gaearon
Copy link
Collaborator

gaearon commented Aug 17, 2020

This has been fixed in React 17.

Fiddle with 16: https://codesandbox.io/s/elegant-wilson-jirq9
Fiddle with 17: https://codesandbox.io/s/nifty-benz-rflo0

There may still be some corner cases so feel free to file new issues if something doesn't work.

@gaearon gaearon closed this as completed Aug 17, 2020
@stevematney
Copy link

This has been fixed in React 17.

@gaearon was #15894 the fix for this?

@gaearon
Copy link
Collaborator

gaearon commented Aug 17, 2020

No, it was the changes to event delegation (to attach events at the root).

https://reactjs.org/blog/2020/08/10/react-v17-rc.html#changes-to-event-delegation
https://github.com/facebook/react/pulls?q=is%3Apr+author%3Atrueadm+modern+event+is%3Amerged

@stevematney
Copy link

That looks great! It seems like the perfect fix. Thank you!

DawsonMacPhee pushed a commit to DawsonMacPhee/researchspace that referenced this issue Nov 24, 2021
…ponents.

This is ugly hack to workaround issue with event propagation when react root is inside custom web component.
See facebook/react#9242

This issue caused strange behaviour of platform components inside semantic-map popovers and Mirador annotation popovers.

Basically even `semantic-link` was not working properly there and instead of changing the content of the page dynamically it reloaded the whole page.
Or for example it was not possible to use `mp-event-trigger` in such context.

Signed-off-by: Artem Kozlov <artem@rem.sh>
@GleanCoder1116
Copy link

升级react 18吧

@kudorgyozo
Copy link

Is this fixed?

@tozz
Copy link

tozz commented Jun 29, 2023

@kudorgyozo If you look five comments up.

This has been fixed in React 17.

Fiddle with 16: https://codesandbox.io/s/elegant-wilson-jirq9 Fiddle with 17: https://codesandbox.io/s/nifty-benz-rflo0

There may still be some corner cases so feel free to file new issues if something doesn't work.

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