From 17dc495e1bd0c1188b61e15381c32738b1053746 Mon Sep 17 00:00:00 2001 From: Pierre-Marie Dartus Date: Fri, 14 Jan 2022 06:45:34 +0100 Subject: [PATCH] refactor(engine-core): Simplify props and attrs patching (#2630) --- .../src/3rdparty/snabbdom/types.ts | 2 +- .../@lwc/engine-core/src/framework/api.ts | 13 ++- .../@lwc/engine-core/src/framework/hooks.ts | 83 +++++-------------- .../src/framework/modules/attrs.ts | 43 ++++------ .../framework/modules/computed-class-attr.ts | 28 ++++--- .../framework/modules/computed-style-attr.ts | 18 ++-- .../src/framework/modules/events.ts | 54 ++---------- .../src/framework/modules/props.ts | 31 ++++--- .../framework/modules/static-class-attr.ts | 9 +- .../framework/modules/static-style-attr.ts | 9 +- .../expected.html | 2 +- .../fixtures/attribute-dynamic/expected.html | 2 +- .../attribute-global-html/expected.html | 2 +- .../fixtures/attribute-static/expected.html | 2 +- .../if-conditional-slot/expected.html | 8 +- 15 files changed, 99 insertions(+), 207 deletions(-) diff --git a/packages/@lwc/engine-core/src/3rdparty/snabbdom/types.ts b/packages/@lwc/engine-core/src/3rdparty/snabbdom/types.ts index 242d47d259..661ae8c5c2 100644 --- a/packages/@lwc/engine-core/src/3rdparty/snabbdom/types.ts +++ b/packages/@lwc/engine-core/src/3rdparty/snabbdom/types.ts @@ -70,7 +70,7 @@ export interface VNodeData { classMap?: Record; styleDecls?: Array<[string, string, boolean]>; context?: Record>; - on?: Record; + on?: Record any>; svg?: boolean; } diff --git a/packages/@lwc/engine-core/src/framework/api.ts b/packages/@lwc/engine-core/src/framework/api.ts index 4ebdd55529..42fbe435bd 100644 --- a/packages/@lwc/engine-core/src/framework/api.ts +++ b/packages/@lwc/engine-core/src/framework/api.ts @@ -63,11 +63,8 @@ import { updateNodeHook, insertNodeHook, removeNodeHook, - createElmHook, - updateElmHook, - createCustomElmHook, - updateCustomElmHook, patchChildren, + patchElementPropsAndAttrs, allocateChildrenHook, markAsDynamicChildren, hydrateChildrenHook, @@ -168,10 +165,10 @@ const ElementHook: Hooks = { fallbackElmHook(elm, vnode); vnode.elm = elm; - createElmHook(vnode); + patchElementPropsAndAttrs(null, vnode); }, update: (oldVnode, vnode) => { - updateElmHook(oldVnode, vnode); + patchElementPropsAndAttrs(oldVnode, vnode); patchChildren(vnode.elm!, oldVnode.children, vnode.children); }, insert: (vnode, parentNode, referenceNode) => { @@ -244,10 +241,10 @@ const CustomElementHook: Hooks = { } else if (vnode.ctor !== UpgradableConstructor) { throw new TypeError(`Incorrect Component Constructor`); } - createCustomElmHook(vnode); + patchElementPropsAndAttrs(null, vnode); }, update: (oldVnode, vnode) => { - updateCustomElmHook(oldVnode, vnode); + patchElementPropsAndAttrs(oldVnode, vnode); const vm = getAssociatedVMIfPresent(vnode.elm); if (vm) { // in fallback mode, the allocation will always set children to diff --git a/packages/@lwc/engine-core/src/framework/hooks.ts b/packages/@lwc/engine-core/src/framework/hooks.ts index 6fef49673d..c3f6077f13 100644 --- a/packages/@lwc/engine-core/src/framework/hooks.ts +++ b/packages/@lwc/engine-core/src/framework/hooks.ts @@ -20,19 +20,20 @@ import { getClassList, setText, getAttribute, remove, insert } from '../renderer import { EmptyArray, parseStyleText } from './utils'; import { createVM, getAssociatedVMIfPresent, VM, ShadowMode, RenderMode } from './vm'; import { VNode, VCustomElement, VElement, VNodes } from '../3rdparty/snabbdom/types'; -import modEvents from './modules/events'; -import modAttrs from './modules/attrs'; -import modProps from './modules/props'; -import modComputedClassName from './modules/computed-class-attr'; -import modComputedStyle from './modules/computed-style-attr'; -import modStaticClassName from './modules/static-class-attr'; -import modStaticStyle from './modules/static-style-attr'; import { updateDynamicChildren, updateStaticChildren } from '../3rdparty/snabbdom/snabbdom'; import { patchElementWithRestrictions, unlockDomMutation, lockDomMutation } from './restrictions'; import { getComponentInternalDef } from './def'; import { markComponentAsDirty } from './component'; import { logError } from '../shared/logger'; +import { patchAttributes } from './modules/attrs'; +import { patchProps } from './modules/props'; +import { patchClassAttribute } from './modules/computed-class-attr'; +import { patchStyleAttribute } from './modules/computed-style-attr'; +import { applyEventListeners } from './modules/events'; +import { applyStaticClassAttribute } from './modules/static-class-attr'; +import { applyStaticStyleAttribute } from './modules/static-style-attr'; + function observeElementChildNodes(elm: Element) { (elm as any).$domManual$ = true; } @@ -84,17 +85,19 @@ export function removeNodeHook(vnode: VNode, parentNode: Node) { } } -export function createElmHook(vnode: VElement) { - modEvents.create(vnode); - // Attrs need to be applied to element before props - // IE11 will wipe out value on radio inputs if value - // is set before type=radio. - modAttrs.create(vnode); - modProps.create(vnode); - modStaticClassName.create(vnode); - modStaticStyle.create(vnode); - modComputedClassName.create(vnode); - modComputedStyle.create(vnode); +export function patchElementPropsAndAttrs(oldVnode: VElement | null, vnode: VElement) { + if (isNull(oldVnode)) { + applyEventListeners(vnode); + applyStaticClassAttribute(vnode); + applyStaticStyleAttribute(vnode); + } + + // 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); + patchStyleAttribute(oldVnode, vnode); + patchAttributes(oldVnode, vnode); + patchProps(oldVnode, vnode); } export const enum LWCDOMMode { @@ -102,15 +105,8 @@ export const enum LWCDOMMode { } export function hydrateElmHook(vnode: VElement) { - modEvents.create(vnode); - // Attrs are already on the element. - // modAttrs.create(vnode); - modProps.create(vnode); - // Already set. - // modStaticClassName.create(vnode); - // modStaticStyle.create(vnode); - // modComputedClassName.create(vnode); - // modComputedStyle.create(vnode); + applyEventListeners(vnode); + patchProps(null, vnode); } export function fallbackElmHook(elm: Element, vnode: VElement) { @@ -146,16 +142,6 @@ export function fallbackElmHook(elm: Element, vnode: VElement) { } } -export function updateElmHook(oldVnode: VElement, vnode: VElement) { - // Attrs need to be applied to element before props - // IE11 will wipe out value on radio inputs if value - // is set before type=radio. - modAttrs.update(oldVnode, vnode); - modProps.update(oldVnode, vnode); - modComputedClassName.update(oldVnode, vnode); - modComputedStyle.update(oldVnode, vnode); -} - export function patchChildren(parent: ParentNode, oldCh: VNodes, newCh: VNodes) { if (hasDynamicChildren(newCh)) { updateDynamicChildren(parent, oldCh, newCh); @@ -219,19 +205,6 @@ export function createViewModelHook(elm: HTMLElement, vnode: VCustomElement) { } } -export function createCustomElmHook(vnode: VCustomElement) { - modEvents.create(vnode); - // Attrs need to be applied to element before props - // IE11 will wipe out value on radio inputs if value - // is set before type=radio. - modAttrs.create(vnode); - modProps.create(vnode); - modStaticClassName.create(vnode); - modStaticStyle.create(vnode); - modComputedClassName.create(vnode); - modComputedStyle.create(vnode); -} - export function createChildrenHook(vnode: VElement) { const { elm, children } = vnode; for (let j = 0; j < children.length; ++j) { @@ -417,16 +390,6 @@ export function hydrateChildrenHook(elmChildren: NodeListOf, children } } -export function updateCustomElmHook(oldVnode: VCustomElement, vnode: VCustomElement) { - // Attrs need to be applied to element before props - // IE11 will wipe out value on radio inputs if value - // is set before type=radio. - modAttrs.update(oldVnode, vnode); - modProps.update(oldVnode, vnode); - modComputedClassName.update(oldVnode, vnode); - modComputedStyle.update(oldVnode, vnode); -} - export function removeElmHook(vnode: VElement) { // this method only needs to search on child vnodes from template // to trigger the remove hook just in case some of those children diff --git a/packages/@lwc/engine-core/src/framework/modules/attrs.ts b/packages/@lwc/engine-core/src/framework/modules/attrs.ts index c877be8228..c066e9245b 100644 --- a/packages/@lwc/engine-core/src/framework/modules/attrs.ts +++ b/packages/@lwc/engine-core/src/framework/modules/attrs.ts @@ -5,50 +5,42 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ import { assert, isNull, isUndefined, keys, StringCharCodeAt } from '@lwc/shared'; + import { setAttribute, removeAttribute } from '../../renderer'; +import { VElement } from '../../3rdparty/snabbdom/types'; + import { unlockAttribute, lockAttribute } from '../attributes'; import { EmptyObject } from '../utils'; -import { VElement } from '../../3rdparty/snabbdom/types'; const xlinkNS = 'http://www.w3.org/1999/xlink'; const xmlNS = 'http://www.w3.org/XML/1998/namespace'; const ColonCharCode = 58; -function updateAttrs(oldVnode: VElement, vnode: VElement) { - const { - data: { attrs }, - } = vnode; - +export function patchAttributes(oldVnode: VElement | null, vnode: VElement) { + const { attrs } = vnode.data; if (isUndefined(attrs)) { return; } - let { - data: { attrs: oldAttrs }, - } = oldVnode; + + const oldAttrs = isNull(oldVnode) ? EmptyObject : oldVnode.data.attrs; if (oldAttrs === attrs) { return; } if (process.env.NODE_ENV !== 'production') { assert.invariant( - isUndefined(oldAttrs) || keys(oldAttrs).join(',') === keys(attrs).join(','), + oldAttrs === EmptyObject || keys(oldAttrs).join(',') === keys(attrs).join(','), `vnode.data.attrs cannot change shape.` ); } - const elm = vnode.elm!; - - let key: string; - oldAttrs = isUndefined(oldAttrs) ? EmptyObject : oldAttrs; - - // update modified attributes, add new attributes - // this routine is only useful for data-* attributes in all kind of elements - // and aria-* in standard elements (custom elements will use props for these) - for (key in attrs) { + const { elm } = vnode; + for (const key in attrs) { const cur = attrs[key]; - const old = (oldAttrs as any)[key]; + const old = oldAttrs[key]; + if (old !== cur) { - unlockAttribute(elm, key); + unlockAttribute(elm!, key); if (StringCharCodeAt.call(key, 3) === ColonCharCode) { // Assume xml namespace setAttribute(elm, key, cur as string, xmlNS); @@ -60,14 +52,7 @@ function updateAttrs(oldVnode: VElement, vnode: VElement) { } else { setAttribute(elm, key, cur as string); } - lockAttribute(elm, key); + lockAttribute(elm!, key); } } } - -const emptyVNode = { data: {} } as VElement; - -export default { - create: (vnode: VElement) => updateAttrs(emptyVNode, vnode), - update: updateAttrs, -}; diff --git a/packages/@lwc/engine-core/src/framework/modules/computed-class-attr.ts b/packages/@lwc/engine-core/src/framework/modules/computed-class-attr.ts index cb9aa969c2..e5e3715010 100644 --- a/packages/@lwc/engine-core/src/framework/modules/computed-class-attr.ts +++ b/packages/@lwc/engine-core/src/framework/modules/computed-class-attr.ts @@ -4,11 +4,21 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { create, freeze, isString, isUndefined, StringCharCodeAt, StringSlice } from '@lwc/shared'; +import { + create, + freeze, + isNull, + isString, + isUndefined, + StringCharCodeAt, + StringSlice, +} from '@lwc/shared'; + import { getClassList } from '../../renderer'; -import { EmptyObject, SPACE_CHAR } from '../utils'; import { VElement } from '../../3rdparty/snabbdom/types'; +import { EmptyObject, SPACE_CHAR } from '../utils'; + const classNameToClassMap = create(null); function getMapFromClassName(className: string | undefined): Record { @@ -47,14 +57,13 @@ function getMapFromClassName(className: string | undefined): Record updateClassAttribute(emptyVNode, vnode), - update: updateClassAttribute, -}; diff --git a/packages/@lwc/engine-core/src/framework/modules/computed-style-attr.ts b/packages/@lwc/engine-core/src/framework/modules/computed-style-attr.ts index 53f7fb787e..b746a980b6 100644 --- a/packages/@lwc/engine-core/src/framework/modules/computed-style-attr.ts +++ b/packages/@lwc/engine-core/src/framework/modules/computed-style-attr.ts @@ -4,17 +4,20 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { isString } from '@lwc/shared'; +import { isNull, isString } from '@lwc/shared'; + import { setAttribute, removeAttribute } from '../../renderer'; -import { VNode } from '../../3rdparty/snabbdom/types'; +import { VElement } from '../../3rdparty/snabbdom/types'; // The style property is a string when defined via an expression in the template. -function updateStyleAttribute(oldVnode: VNode, vnode: VNode) { +export function patchStyleAttribute(oldVnode: VElement | null, vnode: VElement) { const { elm, data: { style: newStyle }, } = vnode; - if (oldVnode.data.style === newStyle) { + + const oldStyle = isNull(oldVnode) ? undefined : oldVnode.data.style; + if (oldStyle === newStyle) { return; } @@ -24,10 +27,3 @@ function updateStyleAttribute(oldVnode: VNode, vnode: VNode) { setAttribute(elm, 'style', newStyle); } } - -const emptyVNode = { data: {} } as VNode; - -export default { - create: (vnode: VNode) => updateStyleAttribute(emptyVNode, vnode), - update: updateStyleAttribute, -}; diff --git a/packages/@lwc/engine-core/src/framework/modules/events.ts b/packages/@lwc/engine-core/src/framework/modules/events.ts index a55e586d6d..9c14998821 100644 --- a/packages/@lwc/engine-core/src/framework/modules/events.ts +++ b/packages/@lwc/engine-core/src/framework/modules/events.ts @@ -5,45 +5,11 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ import { isUndefined } from '@lwc/shared'; -import { addEventListener } from '../../renderer'; -import { VNode } from '../../3rdparty/snabbdom/types'; - -function handleEvent(event: Event, vnode: VNode) { - const { type } = event; - const { - data: { on }, - } = vnode; - const handler = on && on[type]; - // call event handler if exists - if (handler) { - handler.call(undefined, event); - } -} - -interface VNodeEventListener extends EventListener { - vnode?: VNode; -} -interface InteractiveVNode extends VNode { - listener: VNodeEventListener | undefined; -} - -function createListener(): EventListener { - return function handler(event: Event) { - handleEvent(event, (handler as VNodeEventListener).vnode as VNode); - }; -} - -function updateAllEventListeners(oldVnode: InteractiveVNode, vnode: InteractiveVNode) { - if (isUndefined(oldVnode.listener)) { - createAllEventListeners(vnode); - } else { - vnode.listener = oldVnode.listener; - vnode.listener.vnode = vnode; - } -} +import { addEventListener } from '../../renderer'; +import { VElement } from '../../3rdparty/snabbdom/types'; -function createAllEventListeners(vnode: VNode) { +export function applyEventListeners(vnode: VElement) { const { elm, data: { on }, @@ -53,16 +19,8 @@ function createAllEventListeners(vnode: VNode) { return; } - const listener: VNodeEventListener = ((vnode as InteractiveVNode).listener = createListener()); - listener.vnode = vnode; - - let name; - for (name in on) { - addEventListener(elm, name, listener); + for (const name in on) { + const handler = on[name]; + addEventListener(elm, name, handler); } } - -export default { - update: updateAllEventListeners, - create: createAllEventListeners, -}; diff --git a/packages/@lwc/engine-core/src/framework/modules/props.ts b/packages/@lwc/engine-core/src/framework/modules/props.ts index db1c960217..e8861ac22c 100644 --- a/packages/@lwc/engine-core/src/framework/modules/props.ts +++ b/packages/@lwc/engine-core/src/framework/modules/props.ts @@ -4,53 +4,50 @@ * SPDX-License-Identifier: MIT * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ -import { assert, isUndefined, keys } from '@lwc/shared'; -import { setProperty, getProperty } from '../../renderer'; +import { assert, isNull, isUndefined, keys } from '@lwc/shared'; + +import { getProperty, setProperty } from '../../renderer'; import { VElement } from '../../3rdparty/snabbdom/types'; +import { EmptyObject } from '../utils'; + function isLiveBindingProp(sel: string, key: string): boolean { // For properties with live bindings, we read values from the DOM element // instead of relying on internally tracked values. return sel === 'input' && (key === 'value' || key === 'checked'); } -function update(oldVnode: VElement, vnode: VElement) { - const props = vnode.data.props; - +export function patchProps(oldVnode: VElement | null, vnode: VElement) { + const { props } = vnode.data; if (isUndefined(props)) { return; } - const oldProps = oldVnode.data.props; + + const oldProps = isNull(oldVnode) ? EmptyObject : oldVnode.data.props; if (oldProps === props) { return; } if (process.env.NODE_ENV !== 'production') { assert.invariant( - isUndefined(oldProps) || keys(oldProps).join(',') === keys(props).join(','), + oldProps === EmptyObject || keys(oldProps).join(',') === keys(props).join(','), 'vnode.data.props cannot change shape.' ); } - const isFirstPatch = isUndefined(oldProps); + const isFirstPatch = isNull(oldVnode); const { elm, sel } = vnode; for (const key in props) { const cur = props[key]; - // if it is the first time this element is patched, or the current value is different to the previous value... + // Set the property if it's the first time is is patched or if the previous property is + // different than the one previously set. if ( isFirstPatch || - cur !== (isLiveBindingProp(sel, key) ? getProperty(elm!, key) : (oldProps as any)[key]) + cur !== (isLiveBindingProp(sel, key) ? getProperty(elm!, key) : oldProps[key]) ) { setProperty(elm!, key, cur); } } } - -const emptyVNode = { data: {} } as VElement; - -export default { - create: (vnode: VElement) => update(emptyVNode, vnode), - update, -}; diff --git a/packages/@lwc/engine-core/src/framework/modules/static-class-attr.ts b/packages/@lwc/engine-core/src/framework/modules/static-class-attr.ts index c1131011b4..fe72e48a05 100644 --- a/packages/@lwc/engine-core/src/framework/modules/static-class-attr.ts +++ b/packages/@lwc/engine-core/src/framework/modules/static-class-attr.ts @@ -5,13 +5,14 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ import { isUndefined } from '@lwc/shared'; + import { getClassList } from '../../renderer'; -import { VNode } from '../../3rdparty/snabbdom/types'; +import { VElement } from '../../3rdparty/snabbdom/types'; // The HTML class property becomes the vnode.data.classMap object when defined as a string in the template. // The compiler takes care of transforming the inline classnames into an object. It's faster to set the // different classnames properties individually instead of via a string. -function createClassAttribute(vnode: VNode) { +export function applyStaticClassAttribute(vnode: VElement) { const { elm, data: { classMap }, @@ -26,7 +27,3 @@ function createClassAttribute(vnode: VNode) { classList.add(name); } } - -export default { - create: createClassAttribute, -}; diff --git a/packages/@lwc/engine-core/src/framework/modules/static-style-attr.ts b/packages/@lwc/engine-core/src/framework/modules/static-style-attr.ts index 27bda471e7..a946ccbca7 100644 --- a/packages/@lwc/engine-core/src/framework/modules/static-style-attr.ts +++ b/packages/@lwc/engine-core/src/framework/modules/static-style-attr.ts @@ -5,13 +5,14 @@ * For full license text, see the LICENSE file in the repo root or https://opensource.org/licenses/MIT */ import { isUndefined } from '@lwc/shared'; + import { setCSSStyleProperty } from '../../renderer'; -import { VNode } from '../../3rdparty/snabbdom/types'; +import { VElement } from '../../3rdparty/snabbdom/types'; // The HTML style property becomes the vnode.data.styleDecls object when defined as a string in the template. // The compiler takes care of transforming the inline style into an object. It's faster to set the // different style properties individually instead of via a string. -function createStyleAttribute(vnode: VNode) { +export function applyStaticStyleAttribute(vnode: VElement) { const { elm, data: { styleDecls }, @@ -26,7 +27,3 @@ function createStyleAttribute(vnode: VNode) { setCSSStyleProperty(elm, prop, value, important); } } - -export default { - create: createStyleAttribute, -}; diff --git a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-component-global-html/expected.html b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-component-global-html/expected.html index 88da2d4781..0c759c6a50 100644 --- a/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-component-global-html/expected.html +++ b/packages/@lwc/engine-server/src/__tests__/fixtures/attribute-component-global-html/expected.html @@ -1,6 +1,6 @@