From 677acd2e96b2590c4d0ee76c4c842c842c32114d Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Thu, 15 Jul 2021 12:43:24 -0700 Subject: [PATCH 1/2] test: VFlags.NO_CHILDREN case --- src/__test__/patch.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/__test__/patch.spec.ts b/src/__test__/patch.spec.ts index e61a3aa542..2ccee19913 100644 --- a/src/__test__/patch.spec.ts +++ b/src/__test__/patch.spec.ts @@ -177,5 +177,8 @@ describe('.patch', () => { const el = document.createElement('div'); patch(el, m('div', undefined, ['foo'], VFlags.ONLY_TEXT_CHILDREN)); expect(el.textContent).toEqual('foo'); + + patch(el, m('div', undefined, [], VFlags.NO_CHILDREN)); + expect(el.childNodes.length).toEqual(0); }); }); From 821d150b86658bb9ac6d49505eada5c90f90e39b Mon Sep 17 00:00:00 2001 From: Aiden Bai Date: Sat, 17 Jul 2021 13:43:05 -0400 Subject: [PATCH 2/2] fix(patch): remove bad edge case for full replacement --- src/__test__/patch.spec.ts | 51 ++++++++++++++++++++++++-------------- src/patch.ts | 20 +++------------ 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/src/__test__/patch.spec.ts b/src/__test__/patch.spec.ts index 2ccee19913..930ea9edbb 100644 --- a/src/__test__/patch.spec.ts +++ b/src/__test__/patch.spec.ts @@ -13,14 +13,11 @@ const h = (tag: string, props?: VProps, ...children: VNode[]) => describe('.patch', () => { it('should patch element with text as children', () => { const el = createElement(h('div', { id: 'el' }, 'foo')); - document.body.appendChild(el); expect(patch(el, h('div', { id: 'el' }, 'bar'))).toEqual( createElement(h('div', { id: 'el' }, 'bar')), ); - expect(document.querySelector('#el')).toEqual( - createElement(h('div', { id: 'el' }, 'bar')), - ); + expect(el).toEqual(createElement(h('div', { id: 'el' }, 'bar'))); expect(patch(el, h('div', { id: 'el', class: 'new' }, 'baz'))).toEqual( createElement(h('div', { id: 'el', class: 'new' }, 'baz')), ); @@ -30,14 +27,13 @@ describe('.patch', () => { it('should patch text', () => { const el = createElement('foo'); - document.body.appendChild(el); - patch(el, 'bar'); - expect(el.textContent).toEqual('bar'); + + expect(patch(el, 'bar', 'foo').nodeValue).toEqual('bar'); }); it('should remove textContent if no children', () => { const el = createElement('foo'); - document.body.appendChild(el); + el.textContent = 'foo'; expect(patch(el, m('div', undefined, undefined, 0)).textContent).toEqual(''); @@ -114,16 +110,19 @@ describe('.patch', () => { const children: string[] = []; const createVNode = () => m('div', undefined, [...children], undefined, [INSERT(0)]); + const prevVNode1 = createVNode(); children.unshift('foo'); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode1); expect(el.childNodes.length).toEqual(1); + const prevVNode2 = createVNode(); children.unshift('bar'); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode2); expect(el.childNodes.length).toEqual(2); + const prevVNode3 = createVNode(); children.unshift('baz'); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode3); expect(el.childNodes.length).toEqual(3); }); @@ -133,12 +132,14 @@ describe('.patch', () => { const children: string[] = ['foo']; const createVNode = () => m('div', undefined, [...children], undefined, [UPDATE(0)]); + const prevVNode1 = createVNode(); children[0] = 'bar'; - patch(el, createVNode()); + patch(el, createVNode(), prevVNode1); expect(el.textContent).toEqual('bar'); + const prevVNode2 = createVNode(); children[0] = 'baz'; - patch(el, createVNode()); + patch(el, createVNode(), prevVNode2); expect(el.textContent).toEqual('baz'); }); @@ -147,12 +148,14 @@ describe('.patch', () => { const children: string[] = []; const createVNode = () => m('div', undefined, [...children], undefined, [INSERT(0)]); + const prevVNode1 = createVNode(); children.unshift('bar'); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode1); expect(el.childNodes.length).toEqual(1); + const prevVNode2 = createVNode(); children.unshift('baz'); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode2); expect(el.childNodes.length).toEqual(2); }); @@ -164,21 +167,31 @@ describe('.patch', () => { const children: string[] = ['foo', 'bar', 'baz']; const createVNode = () => m('div', undefined, [...children], undefined, [DELETE(0)]); + const prevVNode1 = createVNode(); children.splice(0, 1); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode1); expect(el.firstChild!.nodeValue).toEqual('bar'); + const prevVNode2 = createVNode(); children.splice(0, 1); - patch(el, createVNode()); + patch(el, createVNode(), prevVNode2); expect(el.firstChild!.nodeValue).toEqual('baz'); }); it('should shortcut if flags are present', () => { const el = document.createElement('div'); - patch(el, m('div', undefined, ['foo'], VFlags.ONLY_TEXT_CHILDREN)); + patch( + el, + m('div', undefined, ['foo'], VFlags.ONLY_TEXT_CHILDREN), + m('div', undefined, [], VFlags.ONLY_TEXT_CHILDREN), + ); expect(el.textContent).toEqual('foo'); - patch(el, m('div', undefined, [], VFlags.NO_CHILDREN)); + patch( + el, + m('div', undefined, [], VFlags.NO_CHILDREN), + m('div', undefined, ['foo'], VFlags.NO_CHILDREN), + ); expect(el.childNodes.length).toEqual(0); }); }); diff --git a/src/patch.ts b/src/patch.ts index 3a41a3a546..6e9aff1b78 100644 --- a/src/patch.ts +++ b/src/patch.ts @@ -85,14 +85,9 @@ export const patchChildren = ( }; const replaceElementWithVNode = (el: HTMLElement | Text, newVNode: VNode): HTMLElement | Text => { - if (typeof newVNode === 'string') { - el.textContent = newVNode; - return el.firstChild; - } else { - const newElement = createElement(newVNode); - el.replaceWith(newElement); - return newElement; - } + const newElement = createElement(newVNode); + el.replaceWith(newElement); + return newElement; }; /** @@ -121,14 +116,7 @@ export const patch = ( (!(oldVNode)?.key && !(newVNode)?.key) || (oldVNode)?.key !== (newVNode)?.key ) { - if ( - (oldVNode)?.tag !== (newVNode)?.tag && - !(newVNode).children && - !(newVNode).props - ) { - // newVNode has no props/children is replaced because it is generally - // faster to create a empty HTMLElement rather than iteratively/recursively - // remove props/children + if ((oldVNode)?.tag !== (newVNode)?.tag) { return replaceElementWithVNode(el, newVNode); } if (!(el instanceof Text)) {