From 3e2948dfdf9e4098d447a7c44ed152eb253d3d35 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Wed, 26 Oct 2022 18:03:55 -0700 Subject: [PATCH 01/11] feat: htmlAttributeToProperty --- .../src/__tests__/html-attributes.spec.ts | 111 ++++++++++++++---- packages/@lwc/shared/src/html-attributes.ts | 93 +++++++++++---- 2 files changed, 154 insertions(+), 50 deletions(-) diff --git a/packages/@lwc/shared/src/__tests__/html-attributes.spec.ts b/packages/@lwc/shared/src/__tests__/html-attributes.spec.ts index 1ac066523a..22b4181d47 100644 --- a/packages/@lwc/shared/src/__tests__/html-attributes.spec.ts +++ b/packages/@lwc/shared/src/__tests__/html-attributes.spec.ts @@ -4,34 +4,93 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { htmlPropertyToAttribute } from '../html-attributes'; +import { htmlAttributeToProperty, htmlPropertyToAttribute } from '../html-attributes'; -describe('htmlPropertyToAttribute', () => { - test.each([ - // Standard attribute mapping - ['foo', 'foo'], - ['fooBar', 'foo-bar'], - ['fooBarBaz', 'foo-bar-baz'], - ['FooBar', '-foo-bar'], +const propToAttr = [ + // Standard attribute mapping + ['foo', 'foo'], + ['fooBar', 'foo-bar'], + ['fooBarBaz', 'foo-bar-baz'], + ['FooBar', '-foo-bar'], + + // Aria attribute mapping + ['ariaActiveDescendant', 'aria-activedescendant'], + ['ariaAtomic', 'aria-atomic'], + ['ariaAutoComplete', 'aria-autocomplete'], + ['ariaBusy', 'aria-busy'], + ['ariaChecked', 'aria-checked'], + ['ariaColCount', 'aria-colcount'], + ['ariaColIndex', 'aria-colindex'], + ['ariaColSpan', 'aria-colspan'], + ['ariaControls', 'aria-controls'], + ['ariaCurrent', 'aria-current'], + ['ariaDescribedBy', 'aria-describedby'], + ['ariaDetails', 'aria-details'], + ['ariaDisabled', 'aria-disabled'], + ['ariaErrorMessage', 'aria-errormessage'], + ['ariaExpanded', 'aria-expanded'], + ['ariaFlowTo', 'aria-flowto'], + ['ariaHasPopup', 'aria-haspopup'], + ['ariaHidden', 'aria-hidden'], + ['ariaInvalid', 'aria-invalid'], + ['ariaKeyShortcuts', 'aria-keyshortcuts'], + ['ariaLabel', 'aria-label'], + ['ariaLabelledBy', 'aria-labelledby'], + ['ariaLevel', 'aria-level'], + ['ariaLive', 'aria-live'], + ['ariaModal', 'aria-modal'], + ['ariaMultiLine', 'aria-multiline'], + ['ariaMultiSelectable', 'aria-multiselectable'], + ['ariaOrientation', 'aria-orientation'], + ['ariaOwns', 'aria-owns'], + ['ariaPlaceholder', 'aria-placeholder'], + ['ariaPosInSet', 'aria-posinset'], + ['ariaPressed', 'aria-pressed'], + ['ariaReadOnly', 'aria-readonly'], + ['ariaRelevant', 'aria-relevant'], + ['ariaRequired', 'aria-required'], + ['ariaRoleDescription', 'aria-roledescription'], + ['ariaRowCount', 'aria-rowcount'], + ['ariaRowIndex', 'aria-rowindex'], + ['ariaRowSpan', 'aria-rowspan'], + ['ariaSelected', 'aria-selected'], + ['ariaSetSize', 'aria-setsize'], + ['ariaSort', 'aria-sort'], + ['ariaValueMax', 'aria-valuemax'], + ['ariaValueMin', 'aria-valuemin'], + ['ariaValueNow', 'aria-valuenow'], + ['ariaValueText', 'aria-valuetext'], + ['role', 'role'], + + // Special attribute mapping + ['accessKey', 'accesskey'], + ['readOnly', 'readonly'], + ['tabIndex', 'tabindex'], + ['bgColor', 'bgcolor'], + ['colSpan', 'colspan'], + ['rowSpan', 'rowspan'], + ['contentEditable', 'contenteditable'], + ['crossOrigin', 'crossorigin'], + ['dateTime', 'datetime'], + ['formAction', 'formaction'], + ['isMap', 'ismap'], + ['maxLength', 'maxlength'], + ['minLength', 'minlength'], + ['noValidate', 'novalidate'], + ['useMap', 'usemap'], + ['htmlFor', 'for'], +]; + +const attrToProp = propToAttr.map(([prop, attr]) => [attr, prop]); - // Special attribute mapping - ['accessKey', 'accesskey'], - ['readOnly', 'readonly'], - ['tabIndex', 'tabindex'], - ['bgColor', 'bgcolor'], - ['colSpan', 'colspan'], - ['rowSpan', 'rowspan'], - ['contentEditable', 'contenteditable'], - ['crossOrigin', 'crossorigin'], - ['dateTime', 'datetime'], - ['formAction', 'formaction'], - ['isMap', 'ismap'], - ['maxLength', 'maxlength'], - ['minLength', 'minlength'], - ['noValidate', 'novalidate'], - ['useMap', 'usemap'], - ['htmlFor', 'for'], - ])('htmlPropertyToAttribute("%s") returns "%s"', (prop, attr) => { +describe('htmlPropertyToAttribute', () => { + test.each(propToAttr)('htmlPropertyToAttribute("%s") returns "%s"', (prop, attr) => { expect(htmlPropertyToAttribute(prop)).toEqual(attr); }); }); + +describe('htmlAttributeToProperty', () => { + test.each(attrToProp)('htmlAttributeToProperty("%s") returns "%s"', (attr, prop) => { + expect(htmlAttributeToProperty(attr)).toEqual(prop); + }); +}); diff --git a/packages/@lwc/shared/src/html-attributes.ts b/packages/@lwc/shared/src/html-attributes.ts index e28f41230e..24d846e50a 100644 --- a/packages/@lwc/shared/src/html-attributes.ts +++ b/packages/@lwc/shared/src/html-attributes.ts @@ -4,8 +4,10 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { AriaPropNameToAttrNameMap } from './aria'; -import { isUndefined, StringCharCodeAt, StringFromCharCode } from './language'; +import { AriaAttrNameToPropNameMap, AriaPropNameToAttrNameMap } from './aria'; +import { isUndefined, StringCharCodeAt, StringFromCharCode, StringReplace } from './language'; + +const CAMEL_REGEX = /-([a-z])/g; /** * Maps boolean attribute name to supported tags: 'boolean attr name' => Set of allowed tag names @@ -85,34 +87,56 @@ export function isGlobalHtmlAttribute(attrName: string): boolean { return GLOBAL_ATTRIBUTE.has(attrName); } -/** - * Map composed of properties to attributes not following the HTML property to attribute mapping - * convention. - */ -const NO_STANDARD_PROPERTY_ATTRIBUTE_MAPPING = new Map([ - ['accessKey', 'accesskey'], - ['readOnly', 'readonly'], - ['tabIndex', 'tabindex'], - ['bgColor', 'bgcolor'], - ['colSpan', 'colspan'], - ['rowSpan', 'rowspan'], - ['contentEditable', 'contenteditable'], - ['crossOrigin', 'crossorigin'], - ['dateTime', 'datetime'], - ['formAction', 'formaction'], - ['isMap', 'ismap'], - ['maxLength', 'maxlength'], - ['minLength', 'minlength'], - ['noValidate', 'novalidate'], - ['useMap', 'usemap'], - ['htmlFor', 'for'], -]); +// Convoluted map generation so that @lwc/shared remains fully tree-shakable (verify-treeshakable) +const { NO_STANDARD_ATTRIBUTE_PROPERTY_MAPPING, NO_STANDARD_PROPERTY_ATTRIBUTE_MAPPING } = + /*#__PURE__*/ (() => { + /** + * Map composed of properties to attributes not following the HTML property to attribute mapping + * convention. + */ + const NO_STANDARD_PROPERTY_ATTRIBUTE_MAPPING = new Map([ + ['accessKey', 'accesskey'], + ['readOnly', 'readonly'], + ['tabIndex', 'tabindex'], + ['bgColor', 'bgcolor'], + ['colSpan', 'colspan'], + ['rowSpan', 'rowspan'], + ['contentEditable', 'contenteditable'], + ['crossOrigin', 'crossorigin'], + ['dateTime', 'datetime'], + ['formAction', 'formaction'], + ['isMap', 'ismap'], + ['maxLength', 'maxlength'], + ['minLength', 'minlength'], + ['noValidate', 'novalidate'], + ['useMap', 'usemap'], + ['htmlFor', 'for'], + ]); + + /** + * Inverted map with attribute name key and property name value. + */ + const NO_STANDARD_ATTRIBUTE_PROPERTY_MAPPING = new Map(); + NO_STANDARD_PROPERTY_ATTRIBUTE_MAPPING.forEach((value, key) => + NO_STANDARD_ATTRIBUTE_PROPERTY_MAPPING.set(value, key) + ); + + return { + NO_STANDARD_ATTRIBUTE_PROPERTY_MAPPING, + NO_STANDARD_PROPERTY_ATTRIBUTE_MAPPING, + }; + })(); /** * Map associating previously transformed HTML property into HTML attribute. */ const CACHED_PROPERTY_ATTRIBUTE_MAPPING = new Map(); +/** + * Map associating previously transformed HTML attribute into HTML property. + */ +const CACHED_ATTRIBUTE_PROPERTY_MAPPING = new Map(); + export function htmlPropertyToAttribute(propName: string): string { const ariaAttributeName = AriaPropNameToAttrNameMap[propName]; if (!isUndefined(ariaAttributeName)) { @@ -145,3 +169,24 @@ export function htmlPropertyToAttribute(propName: string): string { CACHED_PROPERTY_ATTRIBUTE_MAPPING.set(propName, attributeName); return attributeName; } + +export function htmlAttributeToProperty(attrName: string): string { + const ariaPropertyName = AriaAttrNameToPropNameMap[attrName]; + if (!isUndefined(ariaPropertyName)) { + return ariaPropertyName; + } + + const specialPropertyName = NO_STANDARD_ATTRIBUTE_PROPERTY_MAPPING.get(attrName); + if (!isUndefined(specialPropertyName)) { + return specialPropertyName; + } + + const cachedPropertyName = CACHED_ATTRIBUTE_PROPERTY_MAPPING.get(attrName); + if (!isUndefined(cachedPropertyName)) { + return cachedPropertyName; + } + + const propertyName = StringReplace.call(attrName, CAMEL_REGEX, (g) => g[1].toUpperCase()); + CACHED_ATTRIBUTE_PROPERTY_MAPPING.set(attrName, propertyName); + return propertyName; +} From 0796f4dc0b2202ea7c7a7138df2d06f39871bbd2 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Fri, 28 Oct 2022 15:30:40 -0700 Subject: [PATCH 02/11] feat: set attr unless prop --- .../src/framework/modules/attr-unless-prop.ts | 56 ++++++++++++++++++ .../engine-core/src/framework/rendering.ts | 58 +++++++++++++++---- .../@lwc/engine-core/src/framework/vnodes.ts | 8 ++- .../test/directive-external/index.spec.js | 0 .../test/directive-external/x/test/test.html | 3 + .../test/directive-external/x/test/test.js | 23 ++++++++ 6 files changed, 137 insertions(+), 11 deletions(-) create mode 100644 packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts create mode 100644 packages/@lwc/integration-karma/test/directive-external/index.spec.js create mode 100644 packages/@lwc/integration-karma/test/directive-external/x/test/test.html create mode 100644 packages/@lwc/integration-karma/test/directive-external/x/test/test.js diff --git a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts new file mode 100644 index 0000000000..7aea6a9f05 --- /dev/null +++ b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts @@ -0,0 +1,56 @@ +import { + htmlAttributeToProperty, + isNull, + isUndefined, + StringCharCodeAt, + XML_NAMESPACE, + XLINK_NAMESPACE, +} from '@lwc/shared'; + +import { unlockAttribute, lockAttribute } from '../attributes'; +import { RendererAPI } from '../renderer'; +import { EmptyObject } from '../utils'; +import { VBaseElement } from '../vnodes'; + +const ColonCharCode = 58; + +export function patchAttrUnlessProp( + oldVnode: VBaseElement | null, + vnode: VBaseElement, + renderer: RendererAPI +) { + const { + data: { attrs }, + elm, + } = vnode; + if (isUndefined(elm) || isUndefined(attrs)) { + return; + } + + const { removeAttribute, setAttribute } = renderer; + const oldAttrs = isNull(oldVnode) ? EmptyObject : oldVnode.data.attrs; + + for (const name in attrs) { + const cur = attrs[name]; + const old = oldAttrs[name]; + + const propName = htmlAttributeToProperty(name); + if (propName in elm) { + (elm as any)[propName] = cur; + } else if (old !== cur) { + unlockAttribute(elm, name); + if (StringCharCodeAt.call(name, 3) === ColonCharCode) { + // Assume xml namespace + setAttribute(elm, name, cur as string, XML_NAMESPACE); + } else if (StringCharCodeAt.call(name, 5) === ColonCharCode) { + // Assume xlink namespace + setAttribute(elm, name, cur as string, XLINK_NAMESPACE); + } else if (isNull(cur) || isUndefined(cur)) { + removeAttribute(elm, name); + } else { + setAttribute(elm, name, cur as string); + } + lockAttribute(elm, name); + } + } +} diff --git a/packages/@lwc/engine-core/src/framework/rendering.ts b/packages/@lwc/engine-core/src/framework/rendering.ts index 2f02293763..57a11f7cf0 100644 --- a/packages/@lwc/engine-core/src/framework/rendering.ts +++ b/packages/@lwc/engine-core/src/framework/rendering.ts @@ -59,9 +59,11 @@ import { VFragment, isVFragment, isVScopedSlotFragment, + VExternalCustomElement, } from './vnodes'; import { patchAttributes } from './modules/attrs'; +import { patchAttrUnlessProp } from './modules/attr-unless-prop'; import { patchProps } from './modules/props'; import { patchClassAttribute } from './modules/computed-class-attr'; import { patchStyleAttribute } from './modules/computed-style-attr'; @@ -121,6 +123,14 @@ function patch(n1: VNode, n2: VNode, parent: ParentNode, renderer: RendererAPI) patchElement(n1 as VElement, n2, n2.data.renderer ?? renderer); break; + case VNodeType.ExternalCustomElement: + patchExternalCustomElement( + n1 as VExternalCustomElement, + n2, + n2.data.renderer ?? renderer + ); + break; + case VNodeType.CustomElement: patchCustomElement(n1 as VCustomElement, n2, parent, n2.data.renderer ?? renderer); break; @@ -263,6 +273,17 @@ function patchElement(n1: VElement, n2: VElement, renderer: RendererAPI) { patchChildren(n1.children, n2.children, elm, renderer); } +function patchExternalCustomElement( + n1: VExternalCustomElement, + n2: VExternalCustomElement, + renderer: RendererAPI +) { + const elm = (n2.elm = n1.elm!); + + patchExternalCustomElementPropsAndAttrs(n1, n2, renderer); + patchChildren(n1.children, n2.children, elm, renderer); +} + function mountStatic( vnode: VStatic, parent: ParentNode, @@ -519,6 +540,23 @@ function insertNode(node: Node, parent: Node, anchor: Node | null, renderer: Ren } } +function patchSpecialAttributes( + oldVnode: VBaseElement | null, + vnode: VBaseElement, + renderer: RendererAPI +) { + if (isNull(oldVnode)) { + applyEventListeners(vnode, renderer); + applyStaticClassAttribute(vnode, renderer); + applyStaticStyleAttribute(vnode, renderer); + } + + // Attrs need to be applied to element before props IE11 will wipe out value on radio inputs if + // value is set before type=radio. + patchClassAttribute(oldVnode, vnode, renderer); + patchStyleAttribute(oldVnode, vnode, renderer); +} + export function removeNode(node: Node, parent: ParentNode, renderer: RendererAPI) { if (process.env.NODE_ENV !== 'production') { unlockDomMutation(); @@ -534,20 +572,20 @@ function patchElementPropsAndAttrs( vnode: VBaseElement, renderer: RendererAPI ) { - if (isNull(oldVnode)) { - applyEventListeners(vnode, renderer); - applyStaticClassAttribute(vnode, renderer); - applyStaticStyleAttribute(vnode, renderer); - } - - // Attrs need to be applied to element before props IE11 will wipe out value on radio inputs if - // value is set before type=radio. - patchClassAttribute(oldVnode, vnode, renderer); - patchStyleAttribute(oldVnode, vnode, renderer); + patchSpecialAttributes(oldVnode, vnode, renderer); patchAttributes(oldVnode, vnode, renderer); patchProps(oldVnode, vnode, renderer); } +function patchExternalCustomElementPropsAndAttrs( + oldVnode: VBaseElement | null, + vnode: VBaseElement, + renderer: RendererAPI +) { + patchSpecialAttributes(oldVnode, vnode, renderer); + patchAttrUnlessProp(oldVnode, vnode, renderer); +} + function applyStyleScoping(elm: Element, owner: VM, renderer: RendererAPI) { // Set the class name for `*.scoped.css` style scoping. const scopeToken = getScopeTokenClass(owner); diff --git a/packages/@lwc/engine-core/src/framework/vnodes.ts b/packages/@lwc/engine-core/src/framework/vnodes.ts index 26755dc0f6..300e8e6b25 100644 --- a/packages/@lwc/engine-core/src/framework/vnodes.ts +++ b/packages/@lwc/engine-core/src/framework/vnodes.ts @@ -14,6 +14,7 @@ export const enum VNodeType { Text, Comment, Element, + ExternalCustomElement, CustomElement, Static, Fragment, @@ -24,11 +25,12 @@ export type VNode = | VText | VComment | VElement + | VExternalCustomElement | VCustomElement | VStatic | VFragment | VScopedSlotFragment; -export type VParentElement = VElement | VCustomElement | VFragment; + export type VNodes = Readonly>; export interface BaseVParent { @@ -92,6 +94,10 @@ export interface VElement extends VBaseElement { type: VNodeType.Element; } +export interface VExternalCustomElement extends VBaseElement { + type: VNodeType.ExternalCustomElement; +} + export interface VCustomElement extends VBaseElement { type: VNodeType.CustomElement; mode: 'closed' | 'open'; diff --git a/packages/@lwc/integration-karma/test/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/directive-external/index.spec.js new file mode 100644 index 0000000000..e69de29bb2 diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html new file mode 100644 index 0000000000..fbe5c0962d --- /dev/null +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js new file mode 100644 index 0000000000..869abea760 --- /dev/null +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js @@ -0,0 +1,23 @@ +import { LightningElement } from 'lwc'; + +class FooBar extends HTMLElement { + static observedAttributes = ['baz']; + attributeChangedCallback(name, oldValue, newValue) { + if (name === 'baz') { + this._baz = `${newValue}-attr`; + } + } + + set baz(value) { + this._baz = `${value}-prop`; + } + get baz() { + return this._baz; + } +} + +export default class Test extends LightningElement { + connectedCallback() { + customElements.define('foo-bar', FooBar); + } +} From 4f4ada95d31220cd20d8e4b9b16004ee29e6b4a0 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Tue, 8 Nov 2022 22:19:01 -0800 Subject: [PATCH 03/11] chore: remove new vnode type in favor of simple approach --- .../engine-core/src/framework/rendering.ts | 64 ++++------------ .../@lwc/engine-core/src/framework/vnodes.ts | 7 +- .../test/directive-external/index.spec.js | 74 +++++++++++++++++++ .../test/directive-external/x/test/test.html | 8 +- .../test/directive-external/x/test/test.js | 39 +++++++--- .../custom-element/expected.js | 2 + .../data-passing/expected.js | 2 + .../template-compiler/src/codegen/index.ts | 6 ++ 8 files changed, 136 insertions(+), 66 deletions(-) diff --git a/packages/@lwc/engine-core/src/framework/rendering.ts b/packages/@lwc/engine-core/src/framework/rendering.ts index 57a11f7cf0..2cfa530dc1 100644 --- a/packages/@lwc/engine-core/src/framework/rendering.ts +++ b/packages/@lwc/engine-core/src/framework/rendering.ts @@ -59,7 +59,6 @@ import { VFragment, isVFragment, isVScopedSlotFragment, - VExternalCustomElement, } from './vnodes'; import { patchAttributes } from './modules/attrs'; @@ -123,14 +122,6 @@ function patch(n1: VNode, n2: VNode, parent: ParentNode, renderer: RendererAPI) patchElement(n1 as VElement, n2, n2.data.renderer ?? renderer); break; - case VNodeType.ExternalCustomElement: - patchExternalCustomElement( - n1 as VExternalCustomElement, - n2, - n2.data.renderer ?? renderer - ); - break; - case VNodeType.CustomElement: patchCustomElement(n1 as VCustomElement, n2, parent, n2.data.renderer ?? renderer); break; @@ -273,17 +264,6 @@ function patchElement(n1: VElement, n2: VElement, renderer: RendererAPI) { patchChildren(n1.children, n2.children, elm, renderer); } -function patchExternalCustomElement( - n1: VExternalCustomElement, - n2: VExternalCustomElement, - renderer: RendererAPI -) { - const elm = (n2.elm = n1.elm!); - - patchExternalCustomElementPropsAndAttrs(n1, n2, renderer); - patchChildren(n1.children, n2.children, elm, renderer); -} - function mountStatic( vnode: VStatic, parent: ParentNode, @@ -540,23 +520,6 @@ function insertNode(node: Node, parent: Node, anchor: Node | null, renderer: Ren } } -function patchSpecialAttributes( - oldVnode: VBaseElement | null, - vnode: VBaseElement, - renderer: RendererAPI -) { - if (isNull(oldVnode)) { - applyEventListeners(vnode, renderer); - applyStaticClassAttribute(vnode, renderer); - applyStaticStyleAttribute(vnode, renderer); - } - - // Attrs need to be applied to element before props IE11 will wipe out value on radio inputs if - // value is set before type=radio. - patchClassAttribute(oldVnode, vnode, renderer); - patchStyleAttribute(oldVnode, vnode, renderer); -} - export function removeNode(node: Node, parent: ParentNode, renderer: RendererAPI) { if (process.env.NODE_ENV !== 'production') { unlockDomMutation(); @@ -572,18 +535,23 @@ function patchElementPropsAndAttrs( vnode: VBaseElement, renderer: RendererAPI ) { - patchSpecialAttributes(oldVnode, vnode, renderer); - patchAttributes(oldVnode, vnode, renderer); - patchProps(oldVnode, vnode, renderer); -} + if (isNull(oldVnode)) { + applyEventListeners(vnode, renderer); + applyStaticClassAttribute(vnode, renderer); + applyStaticStyleAttribute(vnode, renderer); + } -function patchExternalCustomElementPropsAndAttrs( - oldVnode: VBaseElement | null, - vnode: VBaseElement, - renderer: RendererAPI -) { - patchSpecialAttributes(oldVnode, vnode, renderer); - patchAttrUnlessProp(oldVnode, vnode, renderer); + // Attrs need to be applied to element before props IE11 will wipe out value on radio inputs if + // value is set before type=radio. + patchClassAttribute(oldVnode, vnode, renderer); + patchStyleAttribute(oldVnode, vnode, renderer); + + if (vnode.data.external) { + patchAttrUnlessProp(oldVnode, vnode, renderer); + } else { + patchAttributes(oldVnode, vnode, renderer); + patchProps(oldVnode, vnode, renderer); + } } function applyStyleScoping(elm: Element, owner: VM, renderer: RendererAPI) { diff --git a/packages/@lwc/engine-core/src/framework/vnodes.ts b/packages/@lwc/engine-core/src/framework/vnodes.ts index 300e8e6b25..b7ff2be4ee 100644 --- a/packages/@lwc/engine-core/src/framework/vnodes.ts +++ b/packages/@lwc/engine-core/src/framework/vnodes.ts @@ -14,7 +14,6 @@ export const enum VNodeType { Text, Comment, Element, - ExternalCustomElement, CustomElement, Static, Fragment, @@ -25,7 +24,6 @@ export type VNode = | VText | VComment | VElement - | VExternalCustomElement | VCustomElement | VStatic | VFragment @@ -94,10 +92,6 @@ export interface VElement extends VBaseElement { type: VNodeType.Element; } -export interface VExternalCustomElement extends VBaseElement { - type: VNodeType.ExternalCustomElement; -} - export interface VCustomElement extends VBaseElement { type: VNodeType.CustomElement; mode: 'closed' | 'open'; @@ -125,6 +119,7 @@ export interface VNodeData { export interface VElementData extends VNodeData { // Similar to above, all props are readonly readonly key: Key; + readonly external?: boolean; readonly ref?: string; readonly slotData?: any; } diff --git a/packages/@lwc/integration-karma/test/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/directive-external/index.spec.js index e69de29bb2..1fa5971e0e 100644 --- a/packages/@lwc/integration-karma/test/directive-external/index.spec.js +++ b/packages/@lwc/integration-karma/test/directive-external/index.spec.js @@ -0,0 +1,74 @@ +import { createElement } from 'lwc'; +import { isNativeShadowRootInstance } from 'test-utils'; + +import Test from 'x/test'; + +describe('lwc:external', () => { + let test; + + beforeEach(() => { + test = createElement('x-test', { is: Test }); + document.body.appendChild(test); + }); + + describe('when not upgraded', () => { + it('should set only attributes on mount', () => { + return Promise.resolve().then(() => { + const elm = test.shadowRoot.querySelector('foo-upgrade-never'); + expect(elm.getAttribute('foo')).toBe('default'); + expect(elm.foo).toBeUndefined(); + }); + }); + + it('should set only attributes on update', () => { + test.value = 'apple'; + return Promise.resolve().then(() => { + const elm = test.shadowRoot.querySelector('foo-upgrade-never'); + expect(elm.getAttribute('foo')).toBe('apple'); + expect(elm.foo).toBeUndefined(); + }); + }); + }); + + describe('after upgrading', () => { + // This test was not broken up into smaller ones with individual assertions because we + // cannot manage the order in which different tests run. Order is important because once you + // define a custom element, there is no way to undefine it. + it('should set property instead of attribute', async () => { + const elm = test.shadowRoot.querySelector('foo-upgrade-after'); + + test.value = 'sake'; + await Promise.resolve(); + + expect(elm.shadowRoot).toBeNull(); + expect(elm.getAttribute('foo')).toBe('sake'); + expect(elm.foo).toBeUndefined(); + + test.upgrade('foo-upgrade-after'); + + test.value = 'miso'; + await Promise.resolve(); + + expect(elm.getAttribute('foo')).toBe('sake'); + expect(elm.foo).toBe('miso-prop'); + + test.value = 'mirin'; + await Promise.resolve(); + + expect(elm.getAttribute('foo')).toBe('sake'); + expect(elm.foo).toBe('mirin-prop'); + }); + }); + + describe('standard web component apis', () => { + it('should distribute slotted content', () => { + const elm = test.shadowRoot.querySelector('foo-upgrade-before'); + const div = test.shadowRoot.querySelector('div.before'); + const slot = elm.shadowRoot.querySelector('slot'); + + expect(isNativeShadowRootInstance(elm.shadowRoot)).toBeTruthy(); + expect(div.assignedSlot).toBe(slot); + expect(slot.assignedElements().includes(div)).toBeTruthy(); + }); + }); +}); diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html index fbe5c0962d..9b1887b721 100644 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html @@ -1,3 +1,9 @@ diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js index 869abea760..0a2784064a 100644 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js @@ -1,23 +1,40 @@ -import { LightningElement } from 'lwc'; +import { api, LightningElement } from 'lwc'; -class FooBar extends HTMLElement { - static observedAttributes = ['baz']; +customElements.define( + 'foo-upgrade-before', + class FooUpgradeBefore extends HTMLElement { + constructor() { + super(); + this._shadowRoot = this.attachShadow({ mode: 'open' }); + this._shadowRoot.innerHTML = ` + + `; + } + } +); + +class FooUpgradeAfter extends HTMLElement { + static observedAttributes = ['foo']; attributeChangedCallback(name, oldValue, newValue) { - if (name === 'baz') { - this._baz = `${newValue}-attr`; + if (name === 'foo') { + this._foo = `${newValue}-attr`; } } - set baz(value) { - this._baz = `${value}-prop`; + set foo(value) { + this._foo = `${value}-prop`; } - get baz() { - return this._baz; + get foo() { + return this._foo; } } export default class Test extends LightningElement { - connectedCallback() { - customElements.define('foo-bar', FooBar); + @api + value = 'default'; + + @api + upgrade() { + customElements.define('foo-upgrade-after', FooUpgradeAfter); } } diff --git a/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/custom-element/expected.js b/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/custom-element/expected.js index 9f81776ab7..f9dbfb940a 100644 --- a/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/custom-element/expected.js +++ b/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/custom-element/expected.js @@ -5,9 +5,11 @@ const stc0 = { }; const stc1 = { key: 1, + external: true, }; const stc2 = { key: 2, + external: true, }; function tmpl($api, $cmp, $slotset, $ctx) { const { c: api_custom_element, h: api_element } = $api; diff --git a/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/data-passing/expected.js b/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/data-passing/expected.js index 51a0eb2d07..38be2eb654 100644 --- a/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/data-passing/expected.js +++ b/packages/@lwc/template-compiler/src/__tests__/fixtures/directive-external/data-passing/expected.js @@ -14,12 +14,14 @@ function tmpl($api, $cmp, $slotset, $ctx) { ghi: $cmp.jkl, }, key: 1, + external: true, }), api_element("foo-bar", { attrs: { mno: $cmp.pqr, }, key: 2, + external: true, }), ]; /*LWC compiler vX.X.X*/ diff --git a/packages/@lwc/template-compiler/src/codegen/index.ts b/packages/@lwc/template-compiler/src/codegen/index.ts index 9ed83fb983..19c9c36fc0 100644 --- a/packages/@lwc/template-compiler/src/codegen/index.ts +++ b/packages/@lwc/template-compiler/src/codegen/index.ts @@ -30,6 +30,7 @@ import { isSpreadDirective, isElement, isElseifBlock, + isExternalComponent, isScopedSlotFragment, isSlotBindDirective, } from '../shared/ast'; @@ -638,6 +639,11 @@ function transform(codeGen: CodeGen): t.Expression { ) ); } + + if (isExternalComponent(element)) { + data.push(t.property(t.identifier('external'), t.literal(true))); + } + return t.objectExpression(data); } From 57ceebf9ba514ce14f83d51d52311bf5145e65bc Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Wed, 9 Nov 2022 10:30:38 -0800 Subject: [PATCH 04/11] chore: ci feedback --- .../src/framework/modules/attr-unless-prop.ts | 6 ++ .../test/directive-external/index.spec.js | 62 ++++++++++--------- .../test/directive-external/x/test/test.js | 28 +++++---- 3 files changed, 54 insertions(+), 42 deletions(-) diff --git a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts index 7aea6a9f05..2807358b70 100644 --- a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts +++ b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts @@ -1,3 +1,9 @@ +/* + * Copyright (c) 2018, salesforce.com, 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 + */ import { htmlAttributeToProperty, isNull, diff --git a/packages/@lwc/integration-karma/test/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/directive-external/index.spec.js index 1fa5971e0e..b83884bbed 100644 --- a/packages/@lwc/integration-karma/test/directive-external/index.spec.js +++ b/packages/@lwc/integration-karma/test/directive-external/index.spec.js @@ -30,45 +30,47 @@ describe('lwc:external', () => { }); }); - describe('after upgrading', () => { - // This test was not broken up into smaller ones with individual assertions because we - // cannot manage the order in which different tests run. Order is important because once you - // define a custom element, there is no way to undefine it. - it('should set property instead of attribute', async () => { - const elm = test.shadowRoot.querySelector('foo-upgrade-after'); + if (!process.env.COMPAT) { + describe('after upgrading', () => { + // This test was not broken up into smaller ones with individual assertions because we + // cannot manage the order in which different tests run. Order is important because once you + // define a custom element, there is no way to undefine it. + it('should set property instead of attribute', async () => { + const elm = test.shadowRoot.querySelector('foo-upgrade-after'); - test.value = 'sake'; - await Promise.resolve(); + test.value = 'sake'; + await Promise.resolve(); - expect(elm.shadowRoot).toBeNull(); - expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.foo).toBeUndefined(); + expect(elm.shadowRoot).toBeNull(); + expect(elm.getAttribute('foo')).toBe('sake'); + expect(elm.foo).toBeUndefined(); - test.upgrade('foo-upgrade-after'); + test.upgrade('foo-upgrade-after'); - test.value = 'miso'; - await Promise.resolve(); + test.value = 'miso'; + await Promise.resolve(); - expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.foo).toBe('miso-prop'); + expect(elm.getAttribute('foo')).toBe('sake'); + expect(elm.foo).toBe('miso-prop'); - test.value = 'mirin'; - await Promise.resolve(); + test.value = 'mirin'; + await Promise.resolve(); - expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.foo).toBe('mirin-prop'); + expect(elm.getAttribute('foo')).toBe('sake'); + expect(elm.foo).toBe('mirin-prop'); + }); }); - }); - describe('standard web component apis', () => { - it('should distribute slotted content', () => { - const elm = test.shadowRoot.querySelector('foo-upgrade-before'); - const div = test.shadowRoot.querySelector('div.before'); - const slot = elm.shadowRoot.querySelector('slot'); + describe('standard web component apis', () => { + it('should distribute slotted content', () => { + const elm = test.shadowRoot.querySelector('foo-upgrade-before'); + const div = test.shadowRoot.querySelector('div.before'); + const slot = elm.shadowRoot.querySelector('slot'); - expect(isNativeShadowRootInstance(elm.shadowRoot)).toBeTruthy(); - expect(div.assignedSlot).toBe(slot); - expect(slot.assignedElements().includes(div)).toBeTruthy(); + expect(isNativeShadowRootInstance(elm.shadowRoot)).toBeTruthy(); + expect(div.assignedSlot).toBe(slot); + expect(slot.assignedElements().includes(div)).toBeTruthy(); + }); }); - }); + } }); diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js index 0a2784064a..092d3e94d1 100644 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js @@ -1,17 +1,19 @@ import { api, LightningElement } from 'lwc'; -customElements.define( - 'foo-upgrade-before', - class FooUpgradeBefore extends HTMLElement { - constructor() { - super(); - this._shadowRoot = this.attachShadow({ mode: 'open' }); - this._shadowRoot.innerHTML = ` - - `; +if (!process.env.COMPAT) { + customElements.define( + 'foo-upgrade-before', + class FooUpgradeBefore extends HTMLElement { + constructor() { + super(); + this._shadowRoot = this.attachShadow({ mode: 'open' }); + this._shadowRoot.innerHTML = ` + + `; + } } - } -); + ); +} class FooUpgradeAfter extends HTMLElement { static observedAttributes = ['foo']; @@ -35,6 +37,8 @@ export default class Test extends LightningElement { @api upgrade() { - customElements.define('foo-upgrade-after', FooUpgradeAfter); + if (!process.env.COMPAT) { + customElements.define('foo-upgrade-after', FooUpgradeAfter); + } } } From 2a4cce8f44ab837925e5eebcabcc276c89c8c1fb Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Wed, 9 Nov 2022 10:45:55 -0800 Subject: [PATCH 05/11] test: bump bundlesize config by ~5% --- scripts/bundlesize/bundlesize.config.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/bundlesize/bundlesize.config.json b/scripts/bundlesize/bundlesize.config.json index 0a8d219beb..2b9fee32e9 100644 --- a/scripts/bundlesize/bundlesize.config.json +++ b/scripts/bundlesize/bundlesize.config.json @@ -2,7 +2,8 @@ "files": [ { "path": "packages/lwc/dist/engine-dom/umd/es2017/engine-dom.min.js", - "maxSize": "21.0KB" + "maxSize": "21.0KB", + "compression": "gzip" }, { "path": "packages/lwc/dist/synthetic-shadow/umd/es2017/synthetic-shadow.min.js", From 7bc53e77c36c83de40790b8adcddfa8f69aa3ecf Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Fri, 11 Nov 2022 16:55:04 -0800 Subject: [PATCH 06/11] chore: fix compat exception --- .../test/directive-external/x/test/test.js | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js index 092d3e94d1..2b77a6ce45 100644 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js @@ -15,22 +15,6 @@ if (!process.env.COMPAT) { ); } -class FooUpgradeAfter extends HTMLElement { - static observedAttributes = ['foo']; - attributeChangedCallback(name, oldValue, newValue) { - if (name === 'foo') { - this._foo = `${newValue}-attr`; - } - } - - set foo(value) { - this._foo = `${value}-prop`; - } - get foo() { - return this._foo; - } -} - export default class Test extends LightningElement { @api value = 'default'; @@ -38,7 +22,24 @@ export default class Test extends LightningElement { @api upgrade() { if (!process.env.COMPAT) { - customElements.define('foo-upgrade-after', FooUpgradeAfter); + customElements.define( + 'foo-upgrade-after', + class FooUpgradeAfter extends HTMLElement { + static observedAttributes = ['foo']; + attributeChangedCallback(name, oldValue, newValue) { + if (name === 'foo') { + this._foo = `${newValue}-attr`; + } + } + + set foo(value) { + this._foo = `${value}-prop`; + } + get foo() { + return this._foo; + } + } + ); } } } From 15f2e56472778867420f6f8c445142b5210b37cd Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Fri, 11 Nov 2022 20:02:37 -0800 Subject: [PATCH 07/11] chore: review feedback --- .../src/framework/modules/attr-unless-prop.ts | 8 ++--- .../engine-core/src/framework/rendering.ts | 3 +- .../test/directive-external/index.spec.js | 35 +++++++++++++++---- .../test/directive-external/x/test/test.html | 10 +++--- .../test/directive-external/x/test/test.js | 20 ++++++++--- 5 files changed, 56 insertions(+), 20 deletions(-) diff --git a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts index 2807358b70..29ea68bd19 100644 --- a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts +++ b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts @@ -13,7 +13,6 @@ import { XLINK_NAMESPACE, } from '@lwc/shared'; -import { unlockAttribute, lockAttribute } from '../attributes'; import { RendererAPI } from '../renderer'; import { EmptyObject } from '../utils'; import { VBaseElement } from '../vnodes'; @@ -29,7 +28,8 @@ export function patchAttrUnlessProp( data: { attrs }, elm, } = vnode; - if (isUndefined(elm) || isUndefined(attrs)) { + + if (isUndefined(attrs)) { return; } @@ -41,10 +41,9 @@ export function patchAttrUnlessProp( const old = oldAttrs[name]; const propName = htmlAttributeToProperty(name); - if (propName in elm) { + if (propName in elm!) { (elm as any)[propName] = cur; } else if (old !== cur) { - unlockAttribute(elm, name); if (StringCharCodeAt.call(name, 3) === ColonCharCode) { // Assume xml namespace setAttribute(elm, name, cur as string, XML_NAMESPACE); @@ -56,7 +55,6 @@ export function patchAttrUnlessProp( } else { setAttribute(elm, name, cur as string); } - lockAttribute(elm, name); } } } diff --git a/packages/@lwc/engine-core/src/framework/rendering.ts b/packages/@lwc/engine-core/src/framework/rendering.ts index 2cfa530dc1..c9396cf9c2 100644 --- a/packages/@lwc/engine-core/src/framework/rendering.ts +++ b/packages/@lwc/engine-core/src/framework/rendering.ts @@ -550,8 +550,9 @@ function patchElementPropsAndAttrs( patchAttrUnlessProp(oldVnode, vnode, renderer); } else { patchAttributes(oldVnode, vnode, renderer); - patchProps(oldVnode, vnode, renderer); } + + patchProps(oldVnode, vnode, renderer); } function applyStyleScoping(elm: Element, owner: VM, renderer: RendererAPI) { diff --git a/packages/@lwc/integration-karma/test/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/directive-external/index.spec.js index b83884bbed..561d16b0e9 100644 --- a/packages/@lwc/integration-karma/test/directive-external/index.spec.js +++ b/packages/@lwc/integration-karma/test/directive-external/index.spec.js @@ -16,7 +16,7 @@ describe('lwc:external', () => { return Promise.resolve().then(() => { const elm = test.shadowRoot.querySelector('foo-upgrade-never'); expect(elm.getAttribute('foo')).toBe('default'); - expect(elm.foo).toBeUndefined(); + expect(elm.data).toBeUndefined(); }); }); @@ -25,7 +25,7 @@ describe('lwc:external', () => { return Promise.resolve().then(() => { const elm = test.shadowRoot.querySelector('foo-upgrade-never'); expect(elm.getAttribute('foo')).toBe('apple'); - expect(elm.foo).toBeUndefined(); + expect(elm.data).toBeUndefined(); }); }); }); @@ -43,21 +43,21 @@ describe('lwc:external', () => { expect(elm.shadowRoot).toBeNull(); expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.foo).toBeUndefined(); + expect(elm.data).toBeUndefined(); - test.upgrade('foo-upgrade-after'); + test.upgrade(); test.value = 'miso'; await Promise.resolve(); expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.foo).toBe('miso-prop'); + expect(elm.data).toBe('miso-prop'); test.value = 'mirin'; await Promise.resolve(); expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.foo).toBe('mirin-prop'); + expect(elm.data).toBe('mirin-prop'); }); }); @@ -72,5 +72,28 @@ describe('lwc:external', () => { expect(slot.assignedElements().includes(div)).toBeTruthy(); }); }); + + describe('passing objects as data', () => { + it('should be stringified when set as an attribute', async () => { + const elm = test.shadowRoot.querySelector('foo-set-object'); + + test.value = {}; + + return Promise.resolve().then(() => { + expect(elm.getAttribute('attr')).toBe('[object Object]'); + }); + }); + + it('should pass object without stringifying', () => { + const elm = test.shadowRoot.querySelector('foo-set-object'); + + const obj = {}; + test.value = obj; + + return Promise.resolve().then(() => { + expect(elm.prop).toEqual(obj); + }); + }); + }); } }); diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html index 9b1887b721..559e8bbef6 100644 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html @@ -1,9 +1,11 @@ diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js index 2b77a6ce45..133a9521f6 100644 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js +++ b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js @@ -13,6 +13,18 @@ if (!process.env.COMPAT) { } } ); + + customElements.define( + 'foo-set-object', + class FooSetObject extends HTMLElement { + set prop(value) { + this._prop = value; + } + get prop() { + return this._prop; + } + } + ); } export default class Test extends LightningElement { @@ -28,15 +40,15 @@ export default class Test extends LightningElement { static observedAttributes = ['foo']; attributeChangedCallback(name, oldValue, newValue) { if (name === 'foo') { - this._foo = `${newValue}-attr`; + this._data = `${newValue}-attr`; } } set foo(value) { - this._foo = `${value}-prop`; + this._data = `${value}-prop`; } - get foo() { - return this._foo; + get data() { + return this._data; } } ); From 5f5ee164fbdbc0bef1093ef5f92db30234caf41e Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Fri, 11 Nov 2022 22:18:53 -0800 Subject: [PATCH 08/11] test: consolidate --- .../test/directive-external/index.spec.js | 99 ----------------- .../test/directive-external/x/test/test.html | 11 -- .../test/directive-external/x/test/test.js | 57 ---------- .../custom-elements/ce-with-property.js | 13 +++ .../template/directive-external/index.spec.js | 104 +++++++++++++++++- .../x/withChildren/withChildren.html | 4 +- .../withDelayedUpgrade.html | 3 + .../withDelayedUpgrade/withDelayedUpgrade.js | 28 +++++ .../x/withProperty/withProperty.html | 3 + .../x/withProperty/withProperty.js | 5 + .../withUnregisteredWC.html | 4 +- .../withUnregisteredWC/withUnregisteredWC.js | 6 +- 12 files changed, 163 insertions(+), 174 deletions(-) delete mode 100644 packages/@lwc/integration-karma/test/directive-external/index.spec.js delete mode 100644 packages/@lwc/integration-karma/test/directive-external/x/test/test.html delete mode 100644 packages/@lwc/integration-karma/test/directive-external/x/test/test.js create mode 100644 packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js create mode 100644 packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.html create mode 100644 packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.js create mode 100644 packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html create mode 100644 packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js diff --git a/packages/@lwc/integration-karma/test/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/directive-external/index.spec.js deleted file mode 100644 index 561d16b0e9..0000000000 --- a/packages/@lwc/integration-karma/test/directive-external/index.spec.js +++ /dev/null @@ -1,99 +0,0 @@ -import { createElement } from 'lwc'; -import { isNativeShadowRootInstance } from 'test-utils'; - -import Test from 'x/test'; - -describe('lwc:external', () => { - let test; - - beforeEach(() => { - test = createElement('x-test', { is: Test }); - document.body.appendChild(test); - }); - - describe('when not upgraded', () => { - it('should set only attributes on mount', () => { - return Promise.resolve().then(() => { - const elm = test.shadowRoot.querySelector('foo-upgrade-never'); - expect(elm.getAttribute('foo')).toBe('default'); - expect(elm.data).toBeUndefined(); - }); - }); - - it('should set only attributes on update', () => { - test.value = 'apple'; - return Promise.resolve().then(() => { - const elm = test.shadowRoot.querySelector('foo-upgrade-never'); - expect(elm.getAttribute('foo')).toBe('apple'); - expect(elm.data).toBeUndefined(); - }); - }); - }); - - if (!process.env.COMPAT) { - describe('after upgrading', () => { - // This test was not broken up into smaller ones with individual assertions because we - // cannot manage the order in which different tests run. Order is important because once you - // define a custom element, there is no way to undefine it. - it('should set property instead of attribute', async () => { - const elm = test.shadowRoot.querySelector('foo-upgrade-after'); - - test.value = 'sake'; - await Promise.resolve(); - - expect(elm.shadowRoot).toBeNull(); - expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.data).toBeUndefined(); - - test.upgrade(); - - test.value = 'miso'; - await Promise.resolve(); - - expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.data).toBe('miso-prop'); - - test.value = 'mirin'; - await Promise.resolve(); - - expect(elm.getAttribute('foo')).toBe('sake'); - expect(elm.data).toBe('mirin-prop'); - }); - }); - - describe('standard web component apis', () => { - it('should distribute slotted content', () => { - const elm = test.shadowRoot.querySelector('foo-upgrade-before'); - const div = test.shadowRoot.querySelector('div.before'); - const slot = elm.shadowRoot.querySelector('slot'); - - expect(isNativeShadowRootInstance(elm.shadowRoot)).toBeTruthy(); - expect(div.assignedSlot).toBe(slot); - expect(slot.assignedElements().includes(div)).toBeTruthy(); - }); - }); - - describe('passing objects as data', () => { - it('should be stringified when set as an attribute', async () => { - const elm = test.shadowRoot.querySelector('foo-set-object'); - - test.value = {}; - - return Promise.resolve().then(() => { - expect(elm.getAttribute('attr')).toBe('[object Object]'); - }); - }); - - it('should pass object without stringifying', () => { - const elm = test.shadowRoot.querySelector('foo-set-object'); - - const obj = {}; - test.value = obj; - - return Promise.resolve().then(() => { - expect(elm.prop).toEqual(obj); - }); - }); - }); - } -}); diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html b/packages/@lwc/integration-karma/test/directive-external/x/test/test.html deleted file mode 100644 index 559e8bbef6..0000000000 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.html +++ /dev/null @@ -1,11 +0,0 @@ - diff --git a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js b/packages/@lwc/integration-karma/test/directive-external/x/test/test.js deleted file mode 100644 index 133a9521f6..0000000000 --- a/packages/@lwc/integration-karma/test/directive-external/x/test/test.js +++ /dev/null @@ -1,57 +0,0 @@ -import { api, LightningElement } from 'lwc'; - -if (!process.env.COMPAT) { - customElements.define( - 'foo-upgrade-before', - class FooUpgradeBefore extends HTMLElement { - constructor() { - super(); - this._shadowRoot = this.attachShadow({ mode: 'open' }); - this._shadowRoot.innerHTML = ` - - `; - } - } - ); - - customElements.define( - 'foo-set-object', - class FooSetObject extends HTMLElement { - set prop(value) { - this._prop = value; - } - get prop() { - return this._prop; - } - } - ); -} - -export default class Test extends LightningElement { - @api - value = 'default'; - - @api - upgrade() { - if (!process.env.COMPAT) { - customElements.define( - 'foo-upgrade-after', - class FooUpgradeAfter extends HTMLElement { - static observedAttributes = ['foo']; - attributeChangedCallback(name, oldValue, newValue) { - if (name === 'foo') { - this._data = `${newValue}-attr`; - } - } - - set foo(value) { - this._data = `${value}-prop`; - } - get data() { - return this._data; - } - } - ); - } - } -} diff --git a/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js b/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js new file mode 100644 index 0000000000..6300874bb1 --- /dev/null +++ b/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js @@ -0,0 +1,13 @@ +if (!process.env.COMPAT) { + customElements.define( + 'ce-with-property', + class extends HTMLElement { + set prop(value) { + this._prop = value; + } + get prop() { + return this._prop; + } + } + ); +} diff --git a/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js index 17d7ac4fd4..92fb8613ac 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js +++ b/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js @@ -1,14 +1,17 @@ import { createElement } from 'lwc'; import XWithoutChildren from 'x/withoutChildren'; import XWithChildren from 'x/withChildren'; +import XWithDeclarativeEvent from 'x/withDeclarativeEvent'; +import XWithDelayedUpgrade from 'x/withDelayedUpgrade'; import XWithDifferentViews from 'x/withDifferentViews'; import XWithImperativeEvent from 'x/withImperativeEvent'; -import XWithDeclarativeEvent from 'x/withDeclarativeEvent'; +import XWithProperty from 'x/withProperty'; import XWithUnregisteredWC from 'x/withUnregisteredWC'; import './custom-elements/ce-without-children'; import './custom-elements/ce-with-children'; import './custom-elements/ce-with-event'; +import './custom-elements/ce-with-property'; if (!process.env.COMPAT) { describe('lwc:external directive basic tests', () => { @@ -87,5 +90,104 @@ if (!process.env.COMPAT) { expect(ce instanceof HTMLElement).toBe(true); expect(customElements.get('ce-not-registered')).toBe(undefined); }); + + it('should distribute slotted content', () => { + const elm = createElement('x-with-children', { is: XWithChildren }); + document.body.appendChild(elm); + + const ce = elm.shadowRoot.querySelector('ce-with-children'); + const div = elm.shadowRoot.querySelector('.slotted'); + const slot = ce.shadowRoot.querySelector('slot'); + + expect(div.assignedSlot).toBe(slot); + expect(slot.assignedElements().includes(div)).toBeTruthy(); + }); + + describe('passing objects as data', () => { + let elm; + + beforeEach(() => { + elm = createElement('x-with-property', { is: XWithProperty }); + document.body.appendChild(elm); + }); + + it('should be stringified when set as an attribute', () => { + elm.data = {}; + + return Promise.resolve().then(() => { + const ce = elm.shadowRoot.querySelector('ce-with-property'); + expect(ce.getAttribute('attr')).toBe('[object Object]'); + }); + }); + + it('should pass object without stringifying', () => { + const obj = {}; + elm.data = obj; + + return Promise.resolve().then(() => { + const ce = elm.shadowRoot.querySelector('ce-with-property'); + expect(ce.prop).toEqual(obj); + }); + }); + }); + + describe('when custom element not upgraded', () => { + let elm; + + beforeEach(() => { + elm = createElement('x-with-unregsitered-wc', { is: XWithUnregisteredWC }); + document.body.appendChild(elm); + }); + + it('should set only attributes on mount', () => { + return Promise.resolve().then(() => { + const ce = elm.shadowRoot.querySelector('ce-not-registered'); + expect(ce.getAttribute('attr')).toBe('default'); + expect(elm.attr).toBeUndefined(); + }); + }); + + it('should set only attributes on update', () => { + elm.data = 'apple'; + return Promise.resolve().then(() => { + const ce = elm.shadowRoot.querySelector('ce-not-registered'); + expect(ce.getAttribute('attr')).toBe('apple'); + expect(ce.attr).toBeUndefined(); + }); + }); + }); + + describe('delayed upgrade', () => { + // This test is not broken up into smaller ones with individual assertions because we + // cannot manage the order in which different tests run. Order is important because once + // you define a custom element, there is no way to undefine it and the assertions meant + // for before the upgrade must run first. + it('should set property instead of attribute', async () => { + const elm = createElement('x-with-delayed-upgrade', { is: XWithDelayedUpgrade }); + document.body.appendChild(elm); + const ce = elm.shadowRoot.querySelector('ce-with-delayed-upgrade'); + + elm.data = 'sake'; + await Promise.resolve(); + + expect(ce.shadowRoot).toBeNull(); + expect(ce.getAttribute('foo')).toBe('sake'); + expect(ce.foo).toBeUndefined(); + + elm.upgrade(); + + elm.data = 'miso'; + await Promise.resolve(); + + expect(ce.getAttribute('foo')).toBe('sake'); + expect(ce.foo).toBe('miso-prop'); + + elm.data = 'mirin'; + await Promise.resolve(); + + expect(ce.getAttribute('foo')).toBe('sake'); + expect(ce.foo).toBe('mirin-prop'); + }); + }); }); } diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withChildren/withChildren.html b/packages/@lwc/integration-karma/test/template/directive-external/x/withChildren/withChildren.html index edbef55174..91bf54a46b 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/x/withChildren/withChildren.html +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withChildren/withChildren.html @@ -1,7 +1,7 @@ \ No newline at end of file + diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.html b/packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.html new file mode 100644 index 0000000000..8e15799025 --- /dev/null +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.js b/packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.js new file mode 100644 index 0000000000..73c819049c --- /dev/null +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withDelayedUpgrade/withDelayedUpgrade.js @@ -0,0 +1,28 @@ +import { api, LightningElement } from 'lwc'; + +export default class extends LightningElement { + @api data = 'default'; + + @api upgrade() { + if (!process.env.COMPAT) { + customElements.define( + 'ce-with-delayed-upgrade', + class extends HTMLElement { + static observedAttributes = ['foo']; + attributeChangedCallback(name, oldValue, newValue) { + if (name === 'foo') { + this._data = `${newValue}-attr`; + } + } + + set foo(value) { + this._data = `${value}-prop`; + } + get foo() { + return this._data; + } + } + ); + } + } +} diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html new file mode 100644 index 0000000000..cb33b1504e --- /dev/null +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html @@ -0,0 +1,3 @@ + diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js new file mode 100644 index 0000000000..aee494b6a7 --- /dev/null +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js @@ -0,0 +1,5 @@ +import { api, LightningElement } from 'lwc'; + +export default class Test extends LightningElement { + @api data; +} diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.html b/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.html index 4ad972251e..409e164aa5 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.html +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.html @@ -1,5 +1,5 @@ \ No newline at end of file + diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.js b/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.js index 017477d28b..653f8d4d9c 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.js +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withUnregisteredWC/withUnregisteredWC.js @@ -1,3 +1,5 @@ -import { LightningElement } from 'lwc'; +import { api, LightningElement } from 'lwc'; -export default class WithUnregisteredWC extends LightningElement {} +export default class WithUnregisteredWC extends LightningElement { + @api data = 'default'; +} From 1eafaf7ca9001983cb051fbf2c41a7c75ec05ca9 Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Sat, 12 Nov 2022 00:04:34 -0800 Subject: [PATCH 09/11] chore: resolve conflict --- scripts/bundlesize/bundlesize.config.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/scripts/bundlesize/bundlesize.config.json b/scripts/bundlesize/bundlesize.config.json index 2b9fee32e9..0a8d219beb 100644 --- a/scripts/bundlesize/bundlesize.config.json +++ b/scripts/bundlesize/bundlesize.config.json @@ -2,8 +2,7 @@ "files": [ { "path": "packages/lwc/dist/engine-dom/umd/es2017/engine-dom.min.js", - "maxSize": "21.0KB", - "compression": "gzip" + "maxSize": "21.0KB" }, { "path": "packages/lwc/dist/synthetic-shadow/umd/es2017/synthetic-shadow.min.js", From b6f2823396584b353caf5d4aae28c006d0f3a00a Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Sat, 12 Nov 2022 00:22:46 -0800 Subject: [PATCH 10/11] test: spread with external --- .../custom-elements/ce-with-property.js | 14 ++++++++++++++ .../test/template/directive-external/index.spec.js | 12 ++++++++++++ .../x/withProperty/withProperty.html | 2 +- .../x/withProperty/withProperty.js | 6 ++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js b/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js index 6300874bb1..c2fe912a42 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js +++ b/packages/@lwc/integration-karma/test/template/directive-external/custom-elements/ce-with-property.js @@ -8,6 +8,20 @@ if (!process.env.COMPAT) { get prop() { return this._prop; } + + set kyoto(value) { + this._kyoto = `${value} river`; + } + get kyoto() { + return this._kyoto; + } + + set osaka(value) { + this._osaka = `${value} river`; + } + get osaka() { + return this._osaka; + } } ); } diff --git a/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js b/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js index 92fb8613ac..3ae7767eaa 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js +++ b/packages/@lwc/integration-karma/test/template/directive-external/index.spec.js @@ -131,6 +131,18 @@ if (!process.env.COMPAT) { }); }); + it('should work with lwc:spread', () => { + const elm = createElement('x-with-property', { is: XWithProperty }); + document.body.appendChild(elm); + + return Promise.resolve().then(() => { + const ce = elm.shadowRoot.querySelector('ce-with-property'); + expect(ce.kyoto).toBe('kamogawa river'); + expect(ce.osaka).toBe('yodogawa river'); + expect(ce.tokyo).toBe('tamagawa'); + }); + }); + describe('when custom element not upgraded', () => { let elm; diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html index cb33b1504e..7f7b60ac69 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.html @@ -1,3 +1,3 @@ diff --git a/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js index aee494b6a7..7c8c6a1a47 100644 --- a/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js +++ b/packages/@lwc/integration-karma/test/template/directive-external/x/withProperty/withProperty.js @@ -2,4 +2,10 @@ import { api, LightningElement } from 'lwc'; export default class Test extends LightningElement { @api data; + + spread = { + kyoto: 'kamogawa', + osaka: 'yodogawa', + tokyo: 'tamagawa', + }; } From 8e2ff04e2b9836ebd5069682e642353424deef5a Mon Sep 17 00:00:00 2001 From: Eugene Kashida Date: Mon, 14 Nov 2022 11:50:10 -0800 Subject: [PATCH 11/11] chore: review feedback --- .../src/framework/modules/attr-unless-prop.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts index 29ea68bd19..5b6907678e 100644 --- a/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts +++ b/packages/@lwc/engine-core/src/framework/modules/attr-unless-prop.ts @@ -40,11 +40,11 @@ export function patchAttrUnlessProp( const cur = attrs[name]; const old = oldAttrs[name]; - const propName = htmlAttributeToProperty(name); - if (propName in elm!) { - (elm as any)[propName] = cur; - } else if (old !== cur) { - if (StringCharCodeAt.call(name, 3) === ColonCharCode) { + if (old !== cur) { + const propName = htmlAttributeToProperty(name); + if (propName in elm!) { + (elm as any)[propName] = cur; + } else if (StringCharCodeAt.call(name, 3) === ColonCharCode) { // Assume xml namespace setAttribute(elm, name, cur as string, XML_NAMESPACE); } else if (StringCharCodeAt.call(name, 5) === ColonCharCode) {