Shadow DOM compatibility #1529
Shadow DOM compatibility #1529
Conversation
|
Tests are failing in Canary, but those seem to be related to new deprecations. Should I attempt fixing them in this PR? |
Failing tests in beta and canary are a known issue. You can just ignore them unless they are failing in a different way than on current master. See here for details: #1466 (comment) |
There was a problem hiding this comment.
Thanks a lot for working on Shadow DOM compatibility. Looks good to me overall. But I haven't much experience with ember-ref-bucket. Would need to look more in detail into it (especially related to autotracking). But maybe @simonihmig can pick it up before. 😸
addon/utils/dom.js
Outdated
| console.log('dom', dom); | ||
| let destinationElement = findElementById(dom, id) || findElemementByIdInShadowDom(context, id); | ||
| // AFJLAJLSFJ AFJ LJ |
There was a problem hiding this comment.
console.log() and the comment looks like left-over from debugging.
|
|
||
| export function findElemementByIdInShadowDom(context, id) { | ||
| const owner = getOwner(context); | ||
| return owner.rootElement.querySelector && owner.rootElement.querySelector(`[id="${id}"]`); |
There was a problem hiding this comment.
Don't we have to guard against rootElement potentially being a selector string? We discussed this in chat, and I believe that was our conclusion, wasn't it?
There was a problem hiding this comment.
Initially yes, but then we stubmled randomly upon owner.rootElement being a string.
So I decided to keep this tiny guard. Does not hurt.
There was a problem hiding this comment.
Ah, ok. I misunderstood the code, should have look at it for a second longer 🤦♂️
So I was basically "complaining" about that not being guarded, as I was expecting some typeof owner.rootElement like check, but I see it is effectively guarded. You could refactor this to use optional chaining though...
| return owner.rootElement.querySelector && owner.rootElement.querySelector(`[id="${id}"]`); | |
| return owner.rootElement.querySelector?.(`[id="${id}"]`); |
There was a problem hiding this comment.
Is optional chaining transpiled correctly? I've only used it with TypeScript so far.
There was a problem hiding this comment.
Yes, recent enough versions of ember-cli-babel support it ootb. We use that here frequently!
|
FWIW it seems the |
|
|
No, not at all. But setting properties on |
Not the
It doesn't seem to be listed yet on https://deprecations.emberjs.com/v3.x. |
This PR avoids looking up elements via
documentand id, because such method is unable to pass through the Shadow DOM border.Instead, it uses https://github.com/lifeart/ember-ref-bucket by @lifeart to reference elements directly.