Skip to content

Commit

Permalink
Fix JSX computed not working when returning an array of observables
Browse files Browse the repository at this point in the history
  • Loading branch information
brianmhunt committed Sep 25, 2018
1 parent 2598253 commit 5610312
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<slot>` elements and JSX play better together
* Fix JSX computed not working when returning an array of observables

## 🎩 `alpha-5` (4 July 2018)

Expand Down
67 changes: 67 additions & 0 deletions packages/utils.jsx/spec/jsxBehaviors.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ import {
observable, observableArray
} from '@tko/observable'

import {
computed
} from '@tko/computed'

import {
NativeProvider
} from '@tko/provider.native'
Expand Down Expand Up @@ -297,7 +301,46 @@ describe('jsx', function () {
const jo = new JsxObserver(jsx, parent)
assert.equal(parent.innerHTML, '<!--{"x":"123"}-->')
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-->')
o('zzz')
assert.equal(parent.innerHTML, 'zzz123<!--O-->')
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, '<r>abc<!--O--></r>')
o('zzz')
assert.equal(parent.innerHTML, '<r>zzz123<!--O--></r>')
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, '<r><w>WW</w><!--O--></r>')
o('zzz')
assert.equal(parent.innerHTML, '<r><v>VV</v><!--O--></r>')
jo.dispose()
})

it('inserts a promise after it resolves', async () => {
Expand Down Expand Up @@ -541,5 +584,29 @@ describe('jsx', function () {
assert.equal(parent.innerHTML, 'xy<!--O-->')
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<!--O-->')
x(true)
assert.equal(parent.innerHTML, 'XY<!--O-->')
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<!--O-->')
x(true)
assert.equal(parent.innerHTML, 'AB<!--O-->')
jo.dispose()
})
})
})
11 changes: 9 additions & 2 deletions packages/utils.jsx/src/JsxObserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <!-- /ko -->
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)
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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 () {
Expand Down

0 comments on commit 5610312

Please sign in to comment.