Skip to content

Commit a0c036b

Browse files
christianalfonichristianalfoni
committed
fix(overmind-components): refactor testing approach and fix issue with keyed elements
1 parent 39fe6cc commit a0c036b

18 files changed

+2490
-807
lines changed

‎packages/node_modules/overmind-components/src/Component.test.tsx‎

Lines changed: 126 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -1,138 +1,149 @@
11
import { Overmind } from 'overmind'
2-
import { testRender } from './testRender'
3-
import { Component, h, useOvermind } from './'
2+
import { mockDocument } from './mockDocument'
3+
import { Component, h, useOvermind, render } from './'
44
import { Null } from './Null'
55

66
describe('COMPONENT', () => {
77
test('should reconcile components', () => {
8-
const app = new Overmind({})
9-
const TestCompA = () => <div>foo</div>
10-
const TestCompB = () => <div>bar</div>
11-
const test = testRender(
12-
app,
13-
<div>
14-
<TestCompA />
15-
</div>
16-
)
17-
const target = test.getTargetElement()
18-
19-
target.children[0].reconcile(
20-
<div>
21-
<TestCompB />
22-
</div>
23-
)
24-
25-
expect(test.getUpdates().length).toBe(10)
26-
expect(target.children[0].children[0].vtree.children[0].el.nodeValue).toBe(
27-
'bar'
28-
)
8+
mockDocument((target, updates) => {
9+
const app = new Overmind({})
10+
const TestCompA = () => <div id="foo">foo</div>
11+
const TestCompB = () => <div id="bar">bar</div>
12+
const parent = render(
13+
app,
14+
<div id="parent">
15+
<TestCompA />
16+
</div>,
17+
target
18+
)
19+
20+
parent.children[0].reconcile(
21+
<div>
22+
<TestCompB />
23+
</div>
24+
)
25+
26+
expect(updates).toMatchSnapshot()
27+
expect(target.innerHTML).toMatchSnapshot()
28+
})
2929
})
30+
3031
test('should reconcile NULL components to component', () => {
31-
const app = new Overmind({})
32-
const TestCompA = () => null
33-
const TestCompB = () => <div>bar</div>
34-
const test = testRender(
35-
app,
36-
<div>
37-
<TestCompA />
38-
</div>
39-
)
40-
const target = test.getTargetElement()
41-
42-
target.children[0].reconcile(
43-
<div>
44-
<TestCompB />
45-
</div>
46-
)
47-
48-
expect(test.getUpdates().length).toBe(6)
49-
expect(target.children[0].children[0].vtree.children[0].el.nodeValue).toBe(
50-
'bar'
51-
)
32+
mockDocument((target, updates) => {
33+
const app = new Overmind({})
34+
const TestCompA = () => null
35+
const TestCompB = () => <div id="bar">bar</div>
36+
const parent = render(
37+
app,
38+
<div id="parent">
39+
<TestCompA />
40+
</div>,
41+
target
42+
)
43+
44+
parent.children[0].reconcile(
45+
<div id="parent">
46+
<TestCompB />
47+
</div>
48+
)
49+
50+
expect(updates).toMatchSnapshot()
51+
expect(target.innerHTML).toMatchSnapshot()
52+
})
5253
})
54+
5355
test('should reconcile components to NULL component', () => {
54-
const app = new Overmind({})
55-
const TestCompA = () => <div>foo</div>
56-
const TestCompB = () => null
57-
const test = testRender(
58-
app,
59-
<div>
60-
<TestCompA />
61-
</div>
62-
)
63-
const target = test.getTargetElement()
64-
65-
target.children[0].reconcile(
66-
<div>
67-
<TestCompB />
68-
</div>
69-
)
70-
71-
expect(test.getUpdates().length).toBe(7)
72-
73-
expect(target.children[0].children[0].vtree).toBeInstanceOf(Null)
56+
mockDocument((target, updates) => {
57+
const app = new Overmind({})
58+
const TestCompA = () => <div id="foo">foo</div>
59+
const TestCompB = () => null
60+
const parent = render(
61+
app,
62+
<div id="parent">
63+
<TestCompA />
64+
</div>,
65+
target
66+
)
67+
68+
parent.children[0].reconcile(
69+
<div id="parent">
70+
<TestCompB />
71+
</div>
72+
)
73+
74+
expect(updates).toMatchSnapshot()
75+
expect(target.innerHTML).toMatchSnapshot()
76+
})
7477
})
78+
7579
test('should reconcile NULL components to NULL component', () => {
76-
const app = new Overmind({})
77-
const TestCompA = () => null
78-
const TestCompB = () => null
79-
const test = testRender(
80-
app,
81-
<div>
82-
<TestCompA />
83-
</div>
84-
)
85-
const target = test.getTargetElement()
86-
87-
target.children[0].reconcile(
88-
<div>
89-
<TestCompB />
90-
</div>
91-
)
92-
93-
expect(test.getUpdates().length).toBe(2)
94-
95-
expect(target.children[0].children[0].vtree).toBeInstanceOf(Null)
80+
mockDocument((target, updates) => {
81+
const app = new Overmind({})
82+
const TestCompA = () => null
83+
const TestCompB = () => null
84+
const parent = render(
85+
app,
86+
<div id="parent">
87+
<TestCompA />
88+
</div>,
89+
target
90+
)
91+
92+
parent.children[0].reconcile(
93+
<div id="parent">
94+
<TestCompB />
95+
</div>
96+
)
97+
98+
expect(updates).toMatchSnapshot()
99+
100+
expect(target.innerHTML).toMatchSnapshot()
101+
})
96102
})
103+
97104
test('should reconcile nested components from within', () => {
98-
const app = new Overmind({})
99-
let renderCount = 0
100-
const TestCompA = () => (renderCount++ ? <span>foo</span> : <div>bar</div>)
101-
const TestCompB = () => <TestCompA />
102-
const test = testRender(app, <TestCompB />)
103-
const target = test.getTargetElement()
104-
const testCompA = target.children[0].vtree
105-
106-
testCompA.reconcile(testCompA, true)
107-
108-
expect(test.getUpdates().length).toBe(8)
109-
expect(target.children[0].vtree.vtree.children[0].el.nodeValue).toBe('foo')
105+
mockDocument((target, updates) => {
106+
const app = new Overmind({})
107+
let renderCount = 0
108+
const TestCompA = () =>
109+
renderCount++ ? <span id="foo">foo</span> : <div id="bar">bar</div>
110+
const TestCompB = () => <TestCompA />
111+
const parent = render(app, <TestCompB />, target)
112+
const testCompA = parent.children[0].vtree
113+
114+
testCompA.reconcile(testCompA, true)
115+
116+
expect(updates).toMatchSnapshot()
117+
expect(target.innerHTML).toMatchSnapshot()
118+
})
110119
})
120+
111121
test('should update on state changes', () => {
112-
const config = {
113-
state: {
114-
foo: true,
115-
},
116-
actions: {
117-
changeFoo({ state }) {
118-
state.foo = false
122+
mockDocument((target, updates) => {
123+
const config = {
124+
state: {
125+
foo: true,
126+
},
127+
actions: {
128+
changeFoo({ state }) {
129+
state.foo = false
130+
},
119131
},
120-
},
121-
}
122-
const app = new Overmind(config)
132+
}
133+
const app = new Overmind(config)
123134

124-
const TestComp: Component = () => {
125-
const { state } = useOvermind<typeof config>()
126-
return state.foo ? <span>foo</span> : <div>bar</div>
127-
}
135+
const TestComp: Component = () => {
136+
const { state } = useOvermind<typeof config>()
137+
return state.foo ? <span>foo</span> : <div>bar</div>
138+
}
128139

129-
const test = testRender(app, <TestComp />)
130-
const target = test.getTargetElement()
140+
render(app, <TestComp />, target)
131141

132-
app.actions.changeFoo()
142+
app.actions.changeFoo()
133143

134-
expect(test.getUpdates().length).toBe(8)
144+
expect(updates).toMatchSnapshot()
135145

136-
expect(target.children[0].vtree.children[0].el.nodeValue).toBe('bar')
146+
expect(target.innerHTML).toMatchSnapshot()
147+
})
137148
})
138149
})

‎packages/node_modules/overmind-components/src/Component.ts‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ export class Component {
3636
vtree: any
3737
isReconciling: any
3838
asyncRender: any
39+
isMounted: boolean = false
3940
static nextComponentId = 0
4041
static current
4142
constructor(tag, props, children) {
@@ -70,7 +71,12 @@ export class Component {
7071
children: this.children.length === 1 ? this.children[0] : this.children,
7172
}) || new Null()
7273
const paths = app.clearTrackState(trackId)
74+
7375
this.mutationListener = app.addFlushListener(paths, (flushId, isAsync) => {
76+
if (!this.isMounted) {
77+
return
78+
}
79+
7480
app.eventHub.emitAsync('component:update', {
7581
componentId: this.componentId,
7682
componentInstanceId: this.componentInstanceId,
@@ -101,6 +107,8 @@ export class Component {
101107
listener()
102108
}
103109

110+
this.isMounted = true
111+
104112
return this
105113
}
106114
addHook(hook) {
@@ -199,11 +207,13 @@ export class Component {
199207
return this.vtree instanceof Component ? this.vtree.getChild() : this.vtree
200208
}
201209
unmount() {
210+
this.isMounted = false
211+
cancelAnimationFrame(this.asyncRender)
212+
this.mutationListener.dispose()
202213
this.vtree.unmount()
203214
for (const listener of this.unmountListeners) {
204215
listener()
205216
}
206-
this.mutationListener.dispose()
207217
this.context.app.eventHub.emitAsync('component:remove', {
208218
componentId: this.componentId,
209219
componentInstanceId: this.componentInstanceId,

0 commit comments

Comments
 (0)