fix(engine): close #849 by implementing move hook (#852)

salesforce · Nov 22, 2018 · 6c05d12 · 6c05d12
1 parent 16528a5
commit 6c05d12
Show file tree

Hide file tree

Showing 8 changed files with 82 additions and 19 deletions.
diff --git a/packages/lwc-engine/src/3rdparty/snabbdom/snabbdom.ts b/packages/lwc-engine/src/3rdparty/snabbdom/snabbdom.ts
@@ -122,7 +122,7 @@ export function updateDynamicChildren(
         } else if (sameVnode(oldStartVnode, newEndVnode)) {
             // Vnode moved right
             patchVnode(oldStartVnode, newEndVnode);
-            newEndVnode.hook.insert(
+            newEndVnode.hook.move(
                 oldStartVnode,
                 parentElm,
                 // TODO: resolve this, but using dot notation for nextSibling for now
@@ -133,7 +133,7 @@ export function updateDynamicChildren(
         } else if (sameVnode(oldEndVnode, newStartVnode)) {
             // Vnode moved left
             patchVnode(oldEndVnode, newStartVnode);
-            newStartVnode.hook.insert(
+            newStartVnode.hook.move(
                 oldEndVnode,
                 parentElm,
                 oldStartVnode.elm as Node,
@@ -175,7 +175,7 @@ export function updateDynamicChildren(
                             newStartVnode,
                         );
                         oldCh[idxInOld] = undefined as any;
-                        newStartVnode.hook.insert(
+                        newStartVnode.hook.move(
                             elmToMove,
                             parentElm,
                             oldStartVnode.elm as Node,

diff --git a/packages/lwc-engine/src/3rdparty/snabbdom/types.ts b/packages/lwc-engine/src/3rdparty/snabbdom/types.ts
@@ -82,13 +82,15 @@ export interface VNodeData {
 
 export type CreateHook = (vNode: VNode) => void;
 export type InsertHook = (vNode: VNode, parentNode: Node, referenceNode: Node | null) => void;
+export type MoveHook = (vNode: VNode, parentNode: Node, referenceNode: Node | null) => void;
 export type UpdateHook = (oldVNode: VNode, vNode: VNode) => void;
 export type RemoveHook = (vNode: VNode, parentNode: Node) => void;
 export type DestroyHook = (vNode: VNode) => void;
 
 export interface Hooks {
   create: CreateHook;
   insert: InsertHook;
+  move: MoveHook;
   update: UpdateHook;
   remove: RemoveHook;
   destroy: DestroyHook;

diff --git a/packages/lwc-engine/src/faux-shadow/shadow-root.ts b/packages/lwc-engine/src/faux-shadow/shadow-root.ts
@@ -117,8 +117,7 @@ export class SyntheticShadowRoot extends DocumentFragment implements ShadowRoot
         return getHost(this).baseURI;
     }
     get isConnected(this: SyntheticShadowRootInterface) {
-        // @ts-ignore remove this after upgrading ts 3.x
-        return getHost(this).isConnected;
+        return (compareDocumentPosition.call(document, getHost(this)) & DOCUMENT_POSITION_CONTAINED_BY) !== 0;
     }
     get host(this: SyntheticShadowRootInterface) {
         return getHost(this);

diff --git a/packages/lwc-engine/src/framework/__tests__/diff.spec.ts b/packages/lwc-engine/src/framework/__tests__/diff.spec.ts
@@ -1,6 +1,5 @@
 import { compileTemplate } from 'test-utils';
-
-import { createElement, LightningElement } from '../main';
+import { createElement, LightningElement, registerDecorators } from '../main';
 
 const adjectives = [
     "pretty", "large", "big", "small", "tall", "short", "long", "handsome", "plain",
@@ -173,7 +172,7 @@ describe('diff algo', () => {
             const template = elm.shadowRoot;
             const [e1, e2, e3] = template.querySelectorAll('x-row');
             elm.rows = store.data;
-            return Promise.resolve(() => {
+            return Promise.resolve().then(() => {
                 expect(template.querySelectorAll('x-row').length).toBe(2000);
                 const [r1, r2, r3] = template.querySelectorAll('x-row');
                 expect(r1).toBe(e1);
@@ -191,7 +190,7 @@ describe('diff algo', () => {
             const template = elm.shadowRoot;
             const [e1, e2, e3] = template.querySelectorAll('x-row');
             elm.rows = [a, c, b];
-            return Promise.resolve(() => {
+            return Promise.resolve().then(() => {
                 expect(template.querySelectorAll('x-row').length).toBe(3);
                 const [r1, r2, r3] = template.querySelectorAll('x-row');
                 expect(r2).toBe(e3);
@@ -208,7 +207,7 @@ describe('diff algo', () => {
             const template = elm.shadowRoot;
             const [e1, e2, e3] = template.querySelectorAll('x-row');
             elm.rows = [c, b, a];
-            return Promise.resolve(() => {
+            return Promise.resolve().then(() => {
                 expect(template.querySelectorAll('x-row').length).toBe(3);
                 const [r1, r2, r3] = template.querySelectorAll('x-row');
                 expect(r1).toBe(e3);
@@ -226,7 +225,7 @@ describe('diff algo', () => {
             const template = elm.shadowRoot;
             const [e1, e2, e3] = template.querySelectorAll('x-row');
             elm.rows = [c, b, d];
-            return Promise.resolve(() => {
+            return Promise.resolve().then(() => {
                 expect(template.querySelectorAll('x-row').length).toBe(3);
                 const [r1, r2, r3] = template.querySelectorAll('x-row');
                 expect(r2).toBe(e2);
@@ -243,8 +242,8 @@ describe('diff algo', () => {
             const template = elm.shadowRoot;
             const [e1, e2] = template.querySelectorAll('x-row');
             elm.rows = [a, b, c, d]; // inserting new items first
-            return Promise.resolve(() => {
-                expect(template.querySelectorAll('x-row').length).toBe(3);
+            return Promise.resolve().then(() => {
+                expect(template.querySelectorAll('x-row').length).toBe(4);
                 const [r1, r2, r3, r4] = template.querySelectorAll('x-row');
                 expect(r3).toBe(e1);
                 expect(r4).toBe(e2);
@@ -260,12 +259,63 @@ describe('diff algo', () => {
             const template = elm.shadowRoot;
             const [e1, e2] = template.querySelectorAll('x-row');
             elm.rows = [a, b, c, d]; // inserting new items at the end
-            return Promise.resolve(() => {
-                expect(template.querySelectorAll('x-row').length).toBe(3);
+            return Promise.resolve().then(() => {
+                expect(template.querySelectorAll('x-row').length).toBe(4);
                 const [r1, r2, r3, r4] = template.querySelectorAll('x-row');
                 expect(r1).toBe(e1);
                 expect(r2).toBe(e2);
             });
         });
     });
+    describe('patching', () => {
+        it('should not patch elements twice', () => {
+            const tableHTML = compileTemplate(`<template>
+                <table>
+                    <template for:each={items} for:item="item">
+                        <tr key={item.id}>
+                            <td>{item.value}</td>
+                        </tr>
+                    </template>
+                </table>
+            </template>`);
+            class App extends LightningElement {
+                items = [
+                    { id: 'a', value: 5 },
+                    { id: 'b', value: 4 },
+                    { id: 'c', value: 1 },
+                    { id: 'd', value: 3 },
+                ];
+                sortDir = 'ASC';
+                sort(dir) {
+                    const clone = Array.from(this.items);
+                    this.sortDir = dir;
+                    clone.sort((a, b) => {
+                        if (this.sortDir === 'DESC') {
+                            return b.value - a.value;
+                        }
+                        return a.value - b.value;
+                    });
+                    this.items = clone;
+                }
+                render() {
+                    return tableHTML;
+                }
+            }
+            registerDecorators(App, {
+                track: { items: 1, sortDir: 1 },
+                publicMethods: ['sort']
+            });
+            const elm = createElement('x-foo', { is: App });
+            document.body.appendChild(elm);
+            expect(elm.shadowRoot.textContent).toBe('5413');
+            elm.sort('DESC');
+            return Promise.resolve().then(() => {
+                expect(elm.shadowRoot.textContent).toBe('5431');
+                elm.sort('ASC');
+                return Promise.resolve().then(() => {
+                    expect(elm.shadowRoot.textContent).toBe('1345');
+                });
+            });
+        });
+    });
 });
diff --git a/packages/lwc-engine/src/framework/__tests__/services.spec.ts b/packages/lwc-engine/src/framework/__tests__/services.spec.ts
@@ -69,7 +69,7 @@ describe('services', () => {
             const elm = createElement('x-foo', { is: MyComponent });
             document.body.appendChild(elm);
             document.body.removeChild(elm);
-            return Promise.resolve(() => {
+            return Promise.resolve().then(() => {
                 expect(r).toBe(1);
                 expect(c).toBe(1);
                 expect(d).toBe(1);

diff --git a/packages/lwc-engine/src/framework/api.ts b/packages/lwc-engine/src/framework/api.ts
@@ -85,6 +85,7 @@ const TextHook: Hooks = {
     },
     update: updateNodeHook,
     insert: insertNodeHook,
+    move: insertNodeHook, // same as insert for text nodes
     remove: removeNodeHook,
     destroy: noop,
 };
@@ -100,6 +101,7 @@ const CommentHook: Hooks = {
     },
     update: updateNodeHook,
     insert: insertNodeHook,
+    move: insertNodeHook, // same as insert for comment nodes
     remove: removeNodeHook,
     destroy: noop,
 };
@@ -132,6 +134,9 @@ const ElementHook: Hooks = {
         insertBefore.call(parentNode, vnode.elm as Element, referenceNode);
         createChildrenHook(vnode);
     },
+    move: (vnode: VElement, parentNode: Node, referenceNode: Node | null) => {
+        insertBefore.call(parentNode, vnode.elm as Element, referenceNode);
+    },
     remove: (vnode: VElement, parentNode: Node) => {
         removeChild.call(parentNode, vnode.elm as Node);
         removeElmHook(vnode);
@@ -168,6 +173,9 @@ const CustomElementHook: Hooks = {
         createChildrenHook(vnode);
         insertCustomElmHook(vnode);
     },
+    move: (vnode: VCustomElement, parentNode: Node, referenceNode: Node | null) => {
+        insertBefore.call(parentNode, vnode.elm as Element, referenceNode);
+    },
     remove: (vnode: VElement, parentNode: Node) => {
         removeChild.call(parentNode, vnode.elm as Node);
         removeElmHook(vnode);

diff --git a/packages/lwc-engine/src/framework/decorators/__tests__/track.spec.ts b/packages/lwc-engine/src/framework/decorators/__tests__/track.spec.ts
@@ -295,19 +295,23 @@ describe('track.ts', () => {
 
         it('should produce a trackable object', () => {
             let counter = 0;
+            const html = compileTemplate(`<template>{foo.bar}</template>`);
             class MyComponent extends LightningElement {
                 constructor() {
                     super();
                     this.foo = track({ bar: 1 });
                 }
+                render() {
+                    return html;
+                }
                 renderedCallback() {
                     this.foo.bar = 2;
                     counter += 1;
                 }
             }
             const elm = createElement('x-foo', { is: MyComponent });
             document.body.appendChild(elm);
-            return Promise.resolve(() => {
+            return Promise.resolve().then(() => {
                 expect(counter).toBe(2); // two rendering phases due to the mutation of this.foo
             });
         });

diff --git a/packages/lwc-engine/src/framework/decorators/register.ts b/packages/lwc-engine/src/framework/decorators/register.ts
@@ -34,7 +34,7 @@ export interface WireHash {
 export interface RegisterDecoratorMeta {
     readonly publicMethods?: string[];
     readonly publicProps?: PropsDef;
-    readonly track?: string[];
+    readonly track?: TrackDef;
     readonly wire?: WireHash;
 }
 
@@ -85,7 +85,7 @@ export function getDecoratorsRegisteredMeta(Ctor: ComponentConstructor): Decorat
     return signedDecoratorToMetaMap.get(Ctor);
 }
 
-function getTrackHash(target: ComponentConstructor, track: string[] | undefined): TrackDef {
+function getTrackHash(target: ComponentConstructor, track: TrackDef | undefined): TrackDef {
     if (isUndefined(track) || getOwnPropertyNames(track).length === 0) {
         return EmptyObject;
     }