From c6da63a0015caa7cace72a0b7df05c30b15e83e3 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Thu, 19 Dec 2024 13:22:17 -0500 Subject: [PATCH 1/3] fix: use correct shadow root @W-17441501 --- packages/@lwc/engine-dom/src/language.ts | 50 +++++++++++++++++++ .../@lwc/engine-dom/src/renderer/index.ts | 9 ++-- .../LightningElement.shadowRoot/index.spec.js | 14 ++++-- .../x/{test/test.js => basic/basic.js} | 0 .../x/correct/correct.html | 3 ++ .../x/correct/correct.js | 9 ++++ 6 files changed, 79 insertions(+), 6 deletions(-) create mode 100644 packages/@lwc/engine-dom/src/language.ts rename packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/{test/test.js => basic/basic.js} (100%) create mode 100644 packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.html create mode 100644 packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.js diff --git a/packages/@lwc/engine-dom/src/language.ts b/packages/@lwc/engine-dom/src/language.ts new file mode 100644 index 0000000000..2aabb937ec --- /dev/null +++ b/packages/@lwc/engine-dom/src/language.ts @@ -0,0 +1,50 @@ +/* + * Copyright (c) 2024, Salesforce, Inc. + * All rights reserved. + * SPDX-License-Identifier: MIT + * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT + */ + +// Like @lwc/shared, but for DOM APIs + +const ElementDescriptors = Object.getOwnPropertyDescriptors(Element.prototype); +/** Binds `fn.call` to `fn` so you don't need `.call`. */ +const uncall = ( + fn: (this: Element, ...args: Args) => Ret +): ((element: Element, ...args: Args) => Ret) => { + return (element, ...args) => fn.apply(element, args); +}; + +export const ElementAttachShadow = uncall(ElementDescriptors.attachShadow.value!); +export const ElementChildren = uncall(ElementDescriptors.children.get!); +export const ElementClassList = uncall(ElementDescriptors.classList.get!); +export const ElementFirstElementChild = uncall(ElementDescriptors.firstElementChild.get!); +export const ElementGetAttribute = uncall(ElementDescriptors.getAttribute.value!); +export const ElementGetAttributeNS = uncall(ElementDescriptors.getAttributeNS.value!); +export const ElementGetBoundingClientRect = uncall(ElementDescriptors.getBoundingClientRect.value!); +export const ElementGetElementsByClassName = uncall( + ElementDescriptors.getElementsByClassName.value! +); +export const ElementGetElementsByTagName = uncall(ElementDescriptors.getElementsByTagName.value!); +export const ElementHasAttribute = uncall(ElementDescriptors.hasAttribute.value!); +export const ElementHasAttributeNS = uncall(ElementDescriptors.hasAttributeNS.value!); +export const ElementId = uncall(ElementDescriptors.id.get!); +export const ElementLastElementChild = uncall(ElementDescriptors.lastElementChild.get!); +export const ElementQuerySelector = uncall(ElementDescriptors.querySelector.value!); +export const ElementQuerySelectorAll = uncall(ElementDescriptors.querySelectorAll.value!); +export const ElementRemoveAttribute = uncall(ElementDescriptors.removeAttribute.value!); +export const ElementRemoveAttributeNS = uncall(ElementDescriptors.removeAttributeNS.value!); +export const ElementSetAttribute = uncall(ElementDescriptors.setAttribute.value!); +export const ElementSetAttributeNS = uncall(ElementDescriptors.setAttributeNS.value!); +export const ElementShadowRoot = uncall(ElementDescriptors.shadowRoot.get!); +export const ElementTagName = uncall(ElementDescriptors.tagName.get!); + +// Present in `HTMLElementTheGoodParts`, but from a superclass of `Element` +// addEventListener +// childNodes +// dispatchEvent +// firstChild +// isConnected +// lastChild +// ownerDocument +// removeEventListener diff --git a/packages/@lwc/engine-dom/src/renderer/index.ts b/packages/@lwc/engine-dom/src/renderer/index.ts index 64d5e66953..a2d2c5fc58 100644 --- a/packages/@lwc/engine-dom/src/renderer/index.ts +++ b/packages/@lwc/engine-dom/src/renderer/index.ts @@ -6,6 +6,7 @@ */ import { assert, isNull, isUndefined } from '@lwc/shared'; +import { ElementAttachShadow, ElementShadowRoot } from '../language'; function cloneNode(node: Node, deep: boolean): Node { return node.cloneNode(deep); @@ -57,10 +58,12 @@ function attachShadow(element: Element, options: ShadowRootInit): ShadowRoot { // 1. upon initial load with an SSR-generated DOM, while in Shadow render mode // 2. when a webapp author places in their static HTML and mounts their // root component with customElement.define('c-app', Ctor) - if (!isNull(element.shadowRoot)) { - return element.shadowRoot; + // see W-17441501 + const shadowRoot = ElementShadowRoot(element); + if (!isNull(shadowRoot)) { + return shadowRoot; } - return element.attachShadow(options); + return ElementAttachShadow(element, options); } function setText(node: Node, content: string): void { diff --git a/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/index.spec.js b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/index.spec.js index 5dbce2a428..d4da25207a 100644 --- a/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/index.spec.js +++ b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/index.spec.js @@ -1,17 +1,25 @@ import { createElement } from 'lwc'; -import XTest from 'x/test'; +import Basic from 'x/basic'; +import Correct from 'x/correct'; it('should always return null when accessing shadowRoot property from within the component', () => { - const el = createElement('x-test', { is: XTest }); + const el = createElement('x-basic', { is: Basic }); document.body.appendChild(el); expect(el.getShadowRoot()).toBe(null); }); it('should be able to access the shadowRoot property from outside the component', () => { - const el = createElement('x-test', { is: XTest }); + const el = createElement('x-basic', { is: Basic }); document.body.appendChild(el); expect(el.shadowRoot).not.toBe(null); expect(el.shadowRoot).toBeInstanceOf(ShadowRoot); }); + +it('uses the correct shadow root - W-17441501', () => { + const el = createElement('x-correct', { is: Correct }); + document.body.appendChild(el); + + expect(el.shadowRoot.textContent).toBe(''); +}); diff --git a/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/test/test.js b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/basic/basic.js similarity index 100% rename from packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/test/test.js rename to packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/basic/basic.js diff --git a/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.html b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.html new file mode 100644 index 0000000000..b633f5df00 --- /dev/null +++ b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.js b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.js new file mode 100644 index 0000000000..f5cfbe9073 --- /dev/null +++ b/packages/@lwc/integration-karma/test/component/LightningElement.shadowRoot/x/correct/correct.js @@ -0,0 +1,9 @@ +import { LightningElement, api } from 'lwc'; + +export default class extends LightningElement { + @api + get shadowRoot() { + return (this._shadowRoot = + this._shadowRoot || document.body.appendChild(document.createElement('p'))); + } +} From 3fcc1abb761e99d745e96edecb04107b390d0435 Mon Sep 17 00:00:00 2001 From: Will Harney Date: Fri, 20 Dec 2024 11:43:36 -0500 Subject: [PATCH 2/3] chore: yagni i guess --- packages/@lwc/engine-dom/src/language.ts | 43 ++----------------- .../@lwc/engine-dom/src/renderer/index.ts | 6 +-- 2 files changed, 6 insertions(+), 43 deletions(-) diff --git a/packages/@lwc/engine-dom/src/language.ts b/packages/@lwc/engine-dom/src/language.ts index 2aabb937ec..d13a30e48f 100644 --- a/packages/@lwc/engine-dom/src/language.ts +++ b/packages/@lwc/engine-dom/src/language.ts @@ -7,44 +7,7 @@ // Like @lwc/shared, but for DOM APIs -const ElementDescriptors = Object.getOwnPropertyDescriptors(Element.prototype); -/** Binds `fn.call` to `fn` so you don't need `.call`. */ -const uncall = ( - fn: (this: Element, ...args: Args) => Ret -): ((element: Element, ...args: Args) => Ret) => { - return (element, ...args) => fn.apply(element, args); -}; +export const ElementDescriptors = Object.getOwnPropertyDescriptors(Element.prototype); -export const ElementAttachShadow = uncall(ElementDescriptors.attachShadow.value!); -export const ElementChildren = uncall(ElementDescriptors.children.get!); -export const ElementClassList = uncall(ElementDescriptors.classList.get!); -export const ElementFirstElementChild = uncall(ElementDescriptors.firstElementChild.get!); -export const ElementGetAttribute = uncall(ElementDescriptors.getAttribute.value!); -export const ElementGetAttributeNS = uncall(ElementDescriptors.getAttributeNS.value!); -export const ElementGetBoundingClientRect = uncall(ElementDescriptors.getBoundingClientRect.value!); -export const ElementGetElementsByClassName = uncall( - ElementDescriptors.getElementsByClassName.value! -); -export const ElementGetElementsByTagName = uncall(ElementDescriptors.getElementsByTagName.value!); -export const ElementHasAttribute = uncall(ElementDescriptors.hasAttribute.value!); -export const ElementHasAttributeNS = uncall(ElementDescriptors.hasAttributeNS.value!); -export const ElementId = uncall(ElementDescriptors.id.get!); -export const ElementLastElementChild = uncall(ElementDescriptors.lastElementChild.get!); -export const ElementQuerySelector = uncall(ElementDescriptors.querySelector.value!); -export const ElementQuerySelectorAll = uncall(ElementDescriptors.querySelectorAll.value!); -export const ElementRemoveAttribute = uncall(ElementDescriptors.removeAttribute.value!); -export const ElementRemoveAttributeNS = uncall(ElementDescriptors.removeAttributeNS.value!); -export const ElementSetAttribute = uncall(ElementDescriptors.setAttribute.value!); -export const ElementSetAttributeNS = uncall(ElementDescriptors.setAttributeNS.value!); -export const ElementShadowRoot = uncall(ElementDescriptors.shadowRoot.get!); -export const ElementTagName = uncall(ElementDescriptors.tagName.get!); - -// Present in `HTMLElementTheGoodParts`, but from a superclass of `Element` -// addEventListener -// childNodes -// dispatchEvent -// firstChild -// isConnected -// lastChild -// ownerDocument -// removeEventListener +export const ElementAttachShadow = ElementDescriptors.attachShadow.value!; +export const ElementShadowRootGetter = ElementDescriptors.shadowRoot.get!; diff --git a/packages/@lwc/engine-dom/src/renderer/index.ts b/packages/@lwc/engine-dom/src/renderer/index.ts index a2d2c5fc58..10558f98c9 100644 --- a/packages/@lwc/engine-dom/src/renderer/index.ts +++ b/packages/@lwc/engine-dom/src/renderer/index.ts @@ -6,7 +6,7 @@ */ import { assert, isNull, isUndefined } from '@lwc/shared'; -import { ElementAttachShadow, ElementShadowRoot } from '../language'; +import { ElementAttachShadow, ElementShadowRootGetter } from '../language'; function cloneNode(node: Node, deep: boolean): Node { return node.cloneNode(deep); @@ -59,11 +59,11 @@ function attachShadow(element: Element, options: ShadowRootInit): ShadowRoot { // 2. when a webapp author places in their static HTML and mounts their // root component with customElement.define('c-app', Ctor) // see W-17441501 - const shadowRoot = ElementShadowRoot(element); + const shadowRoot = ElementShadowRootGetter.call(element); if (!isNull(shadowRoot)) { return shadowRoot; } - return ElementAttachShadow(element, options); + return ElementAttachShadow.call(element, options); } function setText(node: Node, content: string): void { From 92cb5672ccee5258504366a49e2c07827010e28e Mon Sep 17 00:00:00 2001 From: Will Harney Date: Fri, 20 Dec 2024 12:01:56 -0500 Subject: [PATCH 3/3] =?UTF-8?q?chore:=20=F0=9F=9B=A9=EF=B8=8F=F0=9F=93=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/@lwc/engine-dom/src/language.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/@lwc/engine-dom/src/language.ts b/packages/@lwc/engine-dom/src/language.ts index d13a30e48f..7dad880067 100644 --- a/packages/@lwc/engine-dom/src/language.ts +++ b/packages/@lwc/engine-dom/src/language.ts @@ -5,9 +5,11 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ +import { getOwnPropertyDescriptors } from '@lwc/shared'; + // Like @lwc/shared, but for DOM APIs -export const ElementDescriptors = Object.getOwnPropertyDescriptors(Element.prototype); +export const ElementDescriptors = getOwnPropertyDescriptors(Element.prototype); export const ElementAttachShadow = ElementDescriptors.attachShadow.value!; export const ElementShadowRootGetter = ElementDescriptors.shadowRoot.get!;