fix: revert "perf: hoist static objects/arrays in templates (#2589)"

This reverts commit 0ea4856.
salesforce · Jan 5, 2022 · b2859fa · b2859fa
1 parent 0ea4856
commit b2859fa
Show file tree

Hide file tree

Showing 135 changed files with 3,409 additions and 2,774 deletions.
diff --git a/packages/@lwc/engine-core/src/3rdparty/snabbdom/snabbdom.ts b/packages/@lwc/engine-core/src/3rdparty/snabbdom/snabbdom.ts
@@ -31,11 +31,7 @@ function isVNode(vnode: any): vnode is VNode {
     return vnode != null;
 }
 
-function createKeyToOldIdx(
-    children: Readonly<VNodes>,
-    beginIdx: number,
-    endIdx: number
-): KeyToIndexMap {
+function createKeyToOldIdx(children: VNodes, beginIdx: number, endIdx: number): KeyToIndexMap {
     const map: KeyToIndexMap = {};
     let j: number, key: Key | undefined, ch;
     // TODO [#1637]: simplify this by assuming that all vnodes has keys
@@ -54,7 +50,7 @@ function createKeyToOldIdx(
 function addVnodes(
     parentElm: Node,
     before: Node | null,
-    vnodes: Readonly<VNodes>,
+    vnodes: VNodes,
     startIdx: number,
     endIdx: number
 ) {
@@ -67,12 +63,7 @@ function addVnodes(
     }
 }
 
-function removeVnodes(
-    parentElm: Node,
-    vnodes: Readonly<VNodes>,
-    startIdx: number,
-    endIdx: number
-): void {
+function removeVnodes(parentElm: Node, vnodes: VNodes, startIdx: number, endIdx: number): void {
     for (; startIdx <= endIdx; ++startIdx) {
         const ch = vnodes[startIdx];
         // text nodes do not have logic associated to them
@@ -82,11 +73,7 @@ function removeVnodes(
     }
 }
 
-export function updateDynamicChildren(
-    parentElm: Node,
-    oldCh: Readonly<VNodes>,
-    newCh: Readonly<VNodes>
-) {
+export function updateDynamicChildren(parentElm: Node, oldCh: VNodes, newCh: VNodes) {
     let oldStartIdx = 0;
     let newStartIdx = 0;
     let oldEndIdx = oldCh.length - 1;
@@ -152,12 +139,7 @@ export function updateDynamicChildren(
                         newStartVnode.hook.insert(newStartVnode, parentElm, oldStartVnode.elm!);
                     } else {
                         patchVnode(elmToMove, newStartVnode);
-                        // Delete the old child, but copy the array since it is read-only.
-                        // The `oldCh` will be GC'ed after `updateDynamicChildren` is complete,
-                        // so we only care about the `oldCh` object inside this function.
-                        const oldChClone = [...oldCh];
-                        oldChClone[idxInOld] = undefined as any;
-                        oldCh = oldChClone;
+                        oldCh[idxInOld] = undefined as any;
                         newStartVnode.hook.move(elmToMove, parentElm, oldStartVnode.elm!);
                     }
                 }
@@ -182,11 +164,7 @@ export function updateDynamicChildren(
     }
 }
 
-export function updateStaticChildren(
-    parentElm: Node,
-    oldCh: Readonly<VNodes>,
-    newCh: Readonly<VNodes>
-) {
+export function updateStaticChildren(parentElm: Node, oldCh: VNodes, newCh: VNodes) {
     const oldChLength = oldCh.length;
     const newChLength = newCh.length;
 

diff --git a/packages/@lwc/engine-core/src/3rdparty/snabbdom/types.ts b/packages/@lwc/engine-core/src/3rdparty/snabbdom/types.ts
@@ -21,8 +21,8 @@ export type VNodes = Array<VNode | null>;
 
 export interface VNode {
     sel: string | undefined;
-    data: Readonly<VNodeData>;
-    children: Readonly<VNodes> | undefined;
+    data: VNodeData;
+    children: VNodes | undefined;
     elm: Node | undefined;
     parentElm?: Element;
     text: string | undefined;
@@ -33,8 +33,8 @@ export interface VNode {
 
 export interface VElement extends VNode {
     sel: string;
-    data: Readonly<VElementData>;
-    children: Readonly<VNodes>;
+    data: VElementData;
+    children: VNodes;
     elm: Element | undefined;
     text: undefined;
     key: Key;
@@ -44,7 +44,7 @@ export interface VCustomElement extends VElement {
     mode: 'closed' | 'open';
     ctor: any;
     // copy of the last allocated children.
-    aChildren?: Readonly<VNodes>;
+    aChildren?: VNodes;
 }
 
 export interface VText extends VNode {
@@ -63,21 +63,19 @@ export interface VComment extends VNode {
 }
 
 export interface VNodeData {
-    // All props are readonly because VElementData may be shared across VNodes
-    // due to hoisting optimizations
-    readonly props?: Readonly<Record<string, any>>;
-    readonly attrs?: Readonly<Record<string, string | number | boolean>>;
-    readonly className?: string;
-    readonly style?: string;
-    readonly classMap?: Readonly<Record<string, boolean>>;
-    readonly styleDecls?: Readonly<Array<[string, string, boolean]>>;
-    readonly context?: Readonly<Record<string, Record<string, any>>>;
-    readonly on?: Readonly<Record<string, Function>>;
-    readonly svg?: boolean;
+    props?: Record<string, any>;
+    attrs?: Record<string, string | number | boolean>;
+    className?: string;
+    style?: string;
+    classMap?: Record<string, boolean>;
+    styleDecls?: Array<[string, string, boolean]>;
+    context?: Record<string, Record<string, any>>;
+    on?: Record<string, Function>;
+    svg?: boolean;
 }
 
 export interface VElementData extends VNodeData {
-    readonly key: Key;
+    key: Key;
 }
 
 export interface Hooks<N extends VNode> {

diff --git a/packages/@lwc/engine-core/src/framework/api.ts b/packages/@lwc/engine-core/src/framework/api.ts
@@ -26,7 +26,7 @@ import {
 import { logError, logWarn } from '../shared/logger';
 import { invokeEventListener } from './invoker';
 import { getVMBeingRendered } from './template';
-import { cloneAndOmitKey, EmptyArray, EmptyObject } from './utils';
+import { EmptyArray, EmptyObject } from './utils';
 import {
     appendVM,
     getAssociatedVMIfPresent,
@@ -204,11 +204,7 @@ const ElementHook: Hooks<VElement> = {
             const { props } = vnode.data;
             if (!isUndefined(props) && !isUndefined(props.innerHTML)) {
                 if (elm.innerHTML === props.innerHTML) {
-                    // Do a shallow clone since VNodeData may be shared across VNodes due to hoist optimization
-                    vnode.data = {
-                        ...vnode.data,
-                        props: cloneAndOmitKey(props, 'innerHTML'),
-                    };
+                    delete props.innerHTML;
                 } else {
                     logWarn(
                         `Mismatch hydrating element <${elm.tagName.toLowerCase()}>: innerHTML values do not match for element, will recover from the difference`,
@@ -353,7 +349,7 @@ function addVNodeToChildLWC(vnode: VCustomElement) {
 }
 
 // [h]tml node
-function h(sel: string, data: Readonly<VElementData>, children: Readonly<VNodes>): VElement {
+function h(sel: string, data: VElementData, children: VNodes): VElement {
     const vmBeingRendered = getVMBeingRendered()!;
     if (process.env.NODE_ENV !== 'production') {
         assert.isTrue(isString(sel), `h() 1st argument sel must be a string.`);
@@ -432,10 +428,10 @@ function ti(value: any): number {
 // [s]lot element node
 function s(
     slotName: string,
-    data: Readonly<VElementData>,
-    children: Readonly<VNodes>,
+    data: VElementData,
+    children: VNodes,
     slotset: SlotSet | undefined
-): VElement | Readonly<VNodes> {
+): VElement | VNodes {
     if (process.env.NODE_ENV !== 'production') {
         assert.isTrue(isString(slotName), `s() 1st argument slotName must be a string.`);
         assert.isTrue(isObject(data), `s() 2nd argument data must be an object.`);
@@ -795,10 +791,8 @@ function dc(
     // the new vnode key is a mix of idx and compiler key, this is required by the diffing algo
     // to identify different constructors as vnodes with different keys to avoid reusing the
     // element used for previous constructors.
-    // Shallow clone is necessary here becuase VElementData may be shared across VNodes due to
-    // hoisting optimization.
-    const newData = { ...data, key: `dc:${idx}:${data.key}` };
-    return c(sel, Ctor, newData, children);
+    data.key = `dc:${idx}:${data.key}`;
+    return c(sel, Ctor, data, children);
 }
 
 /**
@@ -814,7 +808,7 @@ function dc(
  *   - children that are produced by iteration
  *
  */
-function sc(vnodes: Readonly<VNodes>): Readonly<VNodes> {
+function sc(vnodes: VNodes): VNodes {
     if (process.env.NODE_ENV !== 'production') {
         assert.isTrue(isArray(vnodes), 'sc() api can only work with arrays.');
     }

diff --git a/packages/@lwc/engine-core/src/framework/hooks.ts b/packages/@lwc/engine-core/src/framework/hooks.ts
@@ -374,11 +374,7 @@ function throwHydrationError() {
     assert.fail('Server rendered elements do not match client side generated elements');
 }
 
-export function hydrateChildrenHook(
-    elmChildren: Readonly<NodeListOf<ChildNode>>,
-    children: Readonly<VNodes>,
-    vm?: VM
-) {
+export function hydrateChildrenHook(elmChildren: NodeListOf<ChildNode>, children: VNodes, vm?: VM) {
     if (process.env.NODE_ENV !== 'production') {
         const filteredVNodes = ArrayFilter.call(children, (vnode) => !!vnode);
 
@@ -452,14 +448,14 @@ export function removeElmHook(vnode: VElement) {
 }
 
 // Using a WeakMap instead of a WeakSet because this one works in IE11 :(
-const FromIteration: WeakMap<Readonly<VNodes>, 1> = new WeakMap();
+const FromIteration: WeakMap<VNodes, 1> = new WeakMap();
 
 // dynamic children means it was generated by an iteration
 // in a template, and will require a more complex diffing algo.
-export function markAsDynamicChildren(children: Readonly<VNodes>) {
+export function markAsDynamicChildren(children: VNodes) {
     FromIteration.set(children, 1);
 }
 
-export function hasDynamicChildren(children: Readonly<VNodes>): boolean {
+export function hasDynamicChildren(children: VNodes): boolean {
     return FromIteration.has(children);
 }
diff --git a/packages/@lwc/engine-core/src/framework/utils.ts b/packages/@lwc/engine-core/src/framework/utils.ts
@@ -74,14 +74,3 @@ export function parseStyleText(cssText: string): { [name: string]: string } {
 
     return styleMap;
 }
-
-// Make a shallow copy of an object but omit the given key
-export function cloneAndOmitKey(object: { [key: string]: any }, keyToOmit: string) {
-    const result: { [key: string]: any } = {};
-    for (const key of Object.keys(object)) {
-        if (key !== keyToOmit) {
-            result[key] = object[key];
-        }
-    }
-    return result;
-}
diff --git a/packages/@lwc/engine-core/src/framework/vm.ts b/packages/@lwc/engine-core/src/framework/vm.ts
@@ -117,9 +117,9 @@ export interface VM<N = HostNode, E = HostElement> {
     /** The component connection state. */
     state: VMState;
     /** The list of VNodes associated with the shadow tree. */
-    children: Readonly<VNodes>;
+    children: VNodes;
     /** The list of adopted children VNodes. */
-    aChildren: Readonly<VNodes>;
+    aChildren: VNodes;
     /** The list of custom elements VNodes currently rendered in the shadow tree. We keep track of
      * those elements to efficiently unmount them when the parent component is disconnected without
      * having to traverse the VNode tree. */
@@ -634,7 +634,7 @@ function runLightChildNodesDisconnectedCallback(vm: VM) {
  * custom element itself will trigger the removal of anything slotted or anything
  * defined on its shadow.
  */
-function recursivelyDisconnectChildren(vnodes: Readonly<VNodes>) {
+function recursivelyDisconnectChildren(vnodes: VNodes) {
     for (let i = 0, len = vnodes.length; i < len; i += 1) {
         const vnode: VCustomElement | VNode | null = vnodes[i];
         if (!isNull(vnode) && isArray(vnode.children) && !isUndefined(vnode.elm)) {
@@ -699,7 +699,7 @@ function getErrorBoundaryVM(vm: VM): VM | undefined {
 // slow path routine
 // NOTE: we should probably more this routine to the synthetic shadow folder
 // and get the allocation to be cached by in the elm instead of in the VM
-export function allocateInSlot(vm: VM, children: Readonly<VNodes>) {
+export function allocateInSlot(vm: VM, children: VNodes) {
     const { cmpSlots: oldSlots } = vm;
     const cmpSlots = (vm.cmpSlots = create(null));
     for (let i = 0, len = children.length; i < len; i += 1) {

diff --git a/packages/@lwc/rollup-plugin/src/__tests__/fixtures/expected_compat_config_simple_app.js b/packages/@lwc/rollup-plugin/src/__tests__/fixtures/expected_compat_config_simple_app.js
@@ -75,15 +75,13 @@
 
   var _implicitStylesheets = [stylesheet];
 
-  var stc0$1 = {
-    key: 0
-  };
-
   function tmpl$1($api, $cmp, $slotset, $ctx) {
     var api_dynamic_text = $api._ES5ProxyType ? $api.get("d") : $api.d,
         api_text = $api._ES5ProxyType ? $api.get("t") : $api.t,
         api_element = $api._ES5ProxyType ? $api.get("h") : $api.h;
-    return [api_element("div", stc0$1, [api_text(api_dynamic_text($cmp._ES5ProxyType ? $cmp.get("x") : $cmp.x))])];
+    return [api_element("div", {
+      key: 0
+    }, [api_text(api_dynamic_text($cmp._ES5ProxyType ? $cmp.get("x") : $cmp.x))])];
   }
 
   var _tmpl$1 = lwc.registerTemplate(tmpl$1);
@@ -136,24 +134,20 @@
     tmpl: _tmpl$1
   });
 
-  var stc0 = {
-    classMap: {
-      "container": true
-    },
-    key: 0
-  };
-  var stc1 = {
-    props: {
-      "x": "1"
-    },
-    key: 1
-  };
-  var stc2 = [];
-
   function tmpl($api, $cmp, $slotset, $ctx) {
     var api_custom_element = $api._ES5ProxyType ? $api.get("c") : $api.c,
         api_element = $api._ES5ProxyType ? $api.get("h") : $api.h;
-    return [api_element("div", stc0, [api_custom_element("x-foo", _xFoo, stc1, stc2)])];
+    return [api_element("div", {
+      classMap: {
+        "container": true
+      },
+      key: 0
+    }, [api_custom_element("x-foo", _xFoo, {
+      props: {
+        "x": "1"
+      },
+      key: 1
+    }, [])])];
   }
 
   var _tmpl = lwc.registerTemplate(tmpl);

diff --git a/packages/@lwc/rollup-plugin/src/__tests__/fixtures/expected_default_config_simple_app.js b/packages/@lwc/rollup-plugin/src/__tests__/fixtures/expected_default_config_simple_app.js
@@ -7,12 +7,11 @@
     }
     var _implicitStylesheets = [stylesheet];
 
-    const stc0$1 = {
-      key: 0
-    };
     function tmpl$1($api, $cmp, $slotset, $ctx) {
       const {d: api_dynamic_text, t: api_text, h: api_element} = $api;
-      return [api_element("div", stc0$1, [api_text(api_dynamic_text($cmp.x))])];
+      return [api_element("div", {
+        key: 0
+      }, [api_text(api_dynamic_text($cmp.x))])];
     }
     var _tmpl$1 = lwc.registerTemplate(tmpl$1);
     tmpl$1.stylesheets = [];
@@ -43,22 +42,19 @@
       tmpl: _tmpl$1
     });
 
-    const stc0 = {
-      classMap: {
-        "container": true
-      },
-      key: 0
-    };
-    const stc1 = {
-      props: {
-        "x": "1"
-      },
-      key: 1
-    };
-    const stc2 = [];
     function tmpl($api, $cmp, $slotset, $ctx) {
       const {c: api_custom_element, h: api_element} = $api;
-      return [api_element("div", stc0, [api_custom_element("x-foo", _xFoo, stc1, stc2)])];
+      return [api_element("div", {
+        classMap: {
+          "container": true
+        },
+        key: 0
+      }, [api_custom_element("x-foo", _xFoo, {
+        props: {
+          "x": "1"
+        },
+        key: 1
+      }, [])])];
     }
     var _tmpl = lwc.registerTemplate(tmpl);
     tmpl.stylesheets = [];