From 56103121cf0615d64c36f8b2667f618e5258e98d Mon Sep 17 00:00:00 2001 From: Brian M Hunt Date: Tue, 25 Sep 2018 13:19:04 -0400 Subject: [PATCH] Fix JSX computed not working when returning an array of observables --- CHANGELOG.md | 1 + packages/utils.jsx/spec/jsxBehaviors.js | 67 +++++++++++++++++++++++++ packages/utils.jsx/src/JsxObserver.js | 11 +++- 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e9e4bd50..3214741e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ For TODO between alpha and release, see https://github.com/knockout/tko/issues/1 * Allow JSX to be used with SVG elements, and respect xmlns attribute * Refresh JSX node when it's subscribable * Make `` elements and JSX play better together +* Fix JSX computed not working when returning an array of observables ## 🎩 `alpha-5` (4 July 2018) diff --git a/packages/utils.jsx/spec/jsxBehaviors.js b/packages/utils.jsx/spec/jsxBehaviors.js index 5381cf42..bea9363f 100644 --- a/packages/utils.jsx/spec/jsxBehaviors.js +++ b/packages/utils.jsx/spec/jsxBehaviors.js @@ -7,6 +7,10 @@ import { observable, observableArray } from '@tko/observable' +import { + computed +} from '@tko/computed' + import { NativeProvider } from '@tko/provider.native' @@ -297,7 +301,46 @@ describe('jsx', function () { const jo = new JsxObserver(jsx, parent) assert.equal(parent.innerHTML, '') jo.dispose() + }) + + it('unwraps multiple text computeds at root', () => { + const parent = document.createElement('div') + const o = observable(false) + const c1 = computed(() => [o, '123']) + const c0 = computed(() => o() ? c1 : 'abc') + const jo = new JsxObserver(c0, parent) + assert.equal(parent.innerHTML, 'abc') + o('zzz') + assert.equal(parent.innerHTML, 'zzz123') + jo.dispose() + }) + it('unwraps multiple text computeds as children', () => { + const parent = document.createElement('div') + const o = observable(false) + const c1 = computed(() => [o, '123']) + const c0 = computed(() => o() ? c1 : 'abc') + const jsx = { elementName: 'r', children: [c0], attributes: {} } + const jo = new JsxObserver(jsx, parent) + assert.equal(parent.innerHTML, 'abc') + o('zzz') + assert.equal(parent.innerHTML, 'zzz123') + jo.dispose() + }) + + it('unwraps multiple element computeds as children', () => { + const parent = document.createElement('div') + const o = observable(false) + const v = { elementName: 'v', children: ['VV'], attributes: {} } + const w = { elementName: 'w', children: ['WW'], attributes: {} } + const c1 = computed(() => v) + const c0 = computed(() => o() ? c1 : w) + const jsx = { elementName: 'r', children: [c0], attributes: {} } + const jo = new JsxObserver(jsx, parent) + assert.equal(parent.innerHTML, 'WW') + o('zzz') + assert.equal(parent.innerHTML, 'VV') + jo.dispose() }) it('inserts a promise after it resolves', async () => { @@ -541,5 +584,29 @@ describe('jsx', function () { assert.equal(parent.innerHTML, 'xy') jo.dispose() }) + + it('removes and adds computed array', () => { + const x = observable(false) + const arr = computed(() => x() ? ['X', 'Y'] : ['Y', 'X']) + const parent = document.createElement('div') + const jo = new JsxObserver(arr, parent) + assert.equal(parent.innerHTML, 'YX') + x(true) + assert.equal(parent.innerHTML, 'XY') + jo.dispose() + }) + + it('removes and adds observables', () => { + const o0 = observable('A') + const o1 = observable('B') + const x = observable(false) + const arr = computed(() => x() ? [o0, o1] : [o0, o1, o0]) + const parent = document.createElement('div') + const jo = new JsxObserver(arr, parent) + assert.equal(parent.innerHTML, 'ABA') + x(true) + assert.equal(parent.innerHTML, 'AB') + jo.dispose() + }) }) }) diff --git a/packages/utils.jsx/src/JsxObserver.js b/packages/utils.jsx/src/JsxObserver.js index 8ddb9b33..3223f91e 100644 --- a/packages/utils.jsx/src/JsxObserver.js +++ b/packages/utils.jsx/src/JsxObserver.js @@ -55,12 +55,16 @@ export class JsxObserver extends LifeCycle { const insertAt = parentNodeIsComment ? parentNode.nextSibling : null insertBefore = document.createComment('O') parentNodeTarget.insertBefore(insertBefore, insertAt) + } else { + this.adoptedInsertBefore = true } } if (parentNodeIsComment && !insertBefore) { // Typcially: insertBefore becomes insertBefore = parentNode.nextSibling + // Mark this so we don't remove the next node - since we didn't create it. + this.adoptedInsertBefore = true } this.anchorTo(insertBefore || parentNode) @@ -96,7 +100,7 @@ export class JsxObserver extends LifeCycle { super.dispose() const ib = this.insertBefore const insertBeforeIsChild = ib && this.parentNodeTarget === ib.parentNode - if (insertBeforeIsChild) { + if (insertBeforeIsChild && !this.adoptedInsertBefore) { this.parentNodeTarget.removeChild(ib) } this.removeAllPriorNodes() @@ -329,7 +333,10 @@ export class JsxObserver extends LifeCycle { lastNodeFor (index) { const nodesAtIndex = this.nodeArrayOrObservableAtIndex[index] || [] const [lastNodeOfPrior] = nodesAtIndex.slice(-1) - return lastNodeOfPrior || this.insertBefore + const insertBefore = lastNodeOfPrior instanceof JsxObserver + ? lastNodeOfPrior.insertBefore : lastNodeOfPrior || this.insertBefore + if (insertBefore) { return insertBefore.parentNode ? insertBefore : null } + return null } removeAllPriorNodes () {