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

htmldomapi.js parentNode does not work with shadowDOM / DocumentFragment #190

Closed
ajmchambers opened this issue Nov 6, 2016 · 4 comments
Closed

Comments

@ajmchambers
Copy link

Hello, I've recently been playing around with shadowDOM and snabbdom and have run into an issue trying to patch an element directly inside a shadowRoot.
(disclaimer: I'm still rather new to github and javascript in general, so do let me know if I'm doing it wrong.)

I'm trying to patch a div inside a shadowRoot as shown below:

<div>
  #shadow-root
    <div></div>
</div>
<div>
  #shadow-root
    <div id="test"></div>
</div>

The reason it is failing seems to be because shadowRoot is a DocumentFragment, and a DocumentFragment's children cannot refer to it using .parentElement.

// DocumentFragment
(function() {
  var div = document.createElement('div');
  var fragment = document.createDocumentFragment();
  fragment.appendChild(div);

  div.parentElement === null; // true
  div.parentNode; // #document-fragment
}());

// shadowDOM
(function() {
  var outerDiv = document.createElement('div');
  var innerDiv = document.createElement('div');
  outerDiv.attachShadow({mode: 'open'});
  var shadowRoot = outerDiv.shadowRoot;
  shadowRoot.appendChild(innerDiv);

  shadowRoot instanceof DocumentFragment; // true
  innerDiv.parentElement === null; // true
  innerDiv.parentNode; // #shadow-root (open)
}());

So I changed the parentNode function in htmldomapi.js to look like below and it now works.

function parentNode(node){
  return node.parentElement || node.parentNode;
}

As I mentioned im still pretty new to all this and I'm not really familiar with the codebase, do you think this would be a safe change to make?

Cheers!

@jbwyme
Copy link

jbwyme commented Nov 22, 2016

👍 We're evaluating snabbdom and we'll need compatibility with ShadowDOM as well. We'll also need support for extending native elements. I'll open up a PR shortly with these changes.

@mreinstein
Copy link
Contributor

I've run into this issue too, where I'd like to attach a shadow dom to a snabbdom element that is being painted into.

@mightyiam mightyiam removed their assignment Dec 17, 2020
@mreinstein mreinstein added this to the v3 milestone Mar 28, 2021
@mreinstein
Copy link
Contributor

I believe this is actually resolved via fa88c0e#diff-b06da8b5eaf15ddfe5341deaeef13cb9c94546db8b0bd927dd358142aa4fc3c7 and this issue can be closed.

@jvanbruegge what do you think?

@jvanbruegge
Copy link
Member

Yes, if not, please open a new issue

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

5 participants