Skip to content

Commit f89d09a

Browse files
Merge pull request #40 from NullVoxPopuli/fix-infinite-rerendering
fix(async functions): properly entangle for the value and prevent inf…
2 parents d8109d5 + ad268fe commit f89d09a

File tree

4 files changed

+259
-212
lines changed

4 files changed

+259
-212
lines changed

addon/-private/resources/function-runner.ts

+33-15
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { isDestroyed, isDestroying } from '@ember/destroyable';
22
import { get as consume, notifyPropertyChange as dirty } from '@ember/object';
3+
import { schedule } from '@ember/runloop';
34
import { waitForPromise } from '@ember/test-waiters';
45

56
import { LifecycleResource } from './lifecycle';
@@ -49,27 +50,44 @@ export class FunctionRunner<
4950
/**
5051
* NOTE: All positional args are consumed
5152
*/
52-
let result = this[FUNCTION_TO_RUN](this.value, ...this.funArgs);
53+
for (let i = 0; i < this.funArgs.length; i++) {
54+
this.funArgs[i];
55+
}
56+
57+
const fun = this[FUNCTION_TO_RUN];
58+
// TS doesn't add the default function symbols to function types...
59+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
60+
const isAsync = (fun as any)[Symbol.toStringTag] === 'AsyncFunction';
61+
/**
62+
* Do not access "value" directly in this function. You'll have infinite re-rendering errors
63+
*/
64+
const previous = this[SECRET_VALUE];
65+
66+
if (isAsync) {
67+
const asyncWaiter = async () => {
68+
// in case the async function tries to consume things on the parent `this`,
69+
// be sure we start with a fresh frame
70+
await new Promise((resolve) => schedule('afterRender', resolve));
71+
72+
if (isDestroying(this) || isDestroyed(this)) {
73+
return;
74+
}
5375

54-
if (typeof result === 'object') {
55-
if ('then' in result) {
56-
const recordValue = (value: Return) => {
57-
if (isDestroying(this) || isDestroyed(this)) {
58-
return;
59-
}
76+
const value = await fun(previous, ...this.funArgs);
6077

61-
this[SECRET_VALUE] = value;
62-
dirty(this, SECRET_VALUE);
63-
};
78+
if (isDestroying(this) || isDestroyed(this)) {
79+
return;
80+
}
6481

65-
waitForPromise(result);
82+
this[SECRET_VALUE] = value;
83+
dirty(this, SECRET_VALUE);
84+
};
6685

67-
result.then(recordValue);
86+
waitForPromise(asyncWaiter());
6887

69-
return;
70-
}
88+
return;
7189
}
7290

73-
this[SECRET_VALUE] = result;
91+
this[SECRET_VALUE] = fun(previous, ...this.funArgs) as Return;
7492
}
7593
}

tests/test-helper.ts

+2
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,6 @@ setApplication(Application.create(config.APP));
1010

1111
setup(QUnit.assert);
1212

13+
QUnit.config.testTimeout = 2000;
14+
1315
start();

tests/unit/use-function-test.ts

+224
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
import Component from '@glimmer/component';
2+
import { tracked } from '@glimmer/tracking';
3+
import { setComponentTemplate } from '@ember/component';
4+
import { destroy } from '@ember/destroyable';
5+
import { click, render, settled } from '@ember/test-helpers';
6+
import { hbs } from 'ember-cli-htmlbars';
7+
import { module, test } from 'qunit';
8+
import { setupRenderingTest, setupTest } from 'ember-qunit';
9+
10+
import { timeout } from 'ember-concurrency';
11+
import { useFunction } from 'ember-resources';
12+
13+
module('useFunction (aliased from useResource)', function () {
14+
module('in js', function (hooks) {
15+
setupTest(hooks);
16+
17+
test('lifecycle', async function (assert) {
18+
let runCount = 0;
19+
20+
class Test {
21+
@tracked count = 1;
22+
23+
data = useFunction(
24+
this,
25+
async () => {
26+
runCount++;
27+
// Pretend we're doing async work
28+
await Promise.resolve();
29+
30+
assert.step(`run ${runCount}`);
31+
},
32+
() => [this.count]
33+
);
34+
}
35+
36+
let foo = new Test();
37+
38+
assert.equal(foo.data.value, undefined);
39+
40+
foo.data.value;
41+
await settled();
42+
foo.count = 2;
43+
foo.data.value;
44+
await settled();
45+
foo.count = 6;
46+
foo.data.value;
47+
destroy(foo); // this prevents a third run
48+
await settled();
49+
50+
assert.verifySteps(['run 1', 'run 2']);
51+
});
52+
53+
test('it works with sync functions', async function (assert) {
54+
class Test {
55+
@tracked count = 1;
56+
57+
data = useFunction(
58+
this,
59+
(previous: number, count: number) => count * (previous || 1),
60+
() => [this.count]
61+
);
62+
}
63+
64+
let foo = new Test();
65+
66+
assert.equal(foo.data.value, 1);
67+
68+
foo.count = 2;
69+
await settled();
70+
71+
assert.equal(foo.data.value, 2);
72+
73+
foo.count = 6;
74+
await settled();
75+
76+
assert.equal(foo.data.value, 12);
77+
});
78+
79+
test('it works with async functions', async function (assert) {
80+
class Test {
81+
@tracked count = 1;
82+
83+
data = useFunction(
84+
this,
85+
async (previous: undefined | number, count: number) => {
86+
// Pretend we're doing async work
87+
await Promise.resolve();
88+
89+
return count * (previous || 1);
90+
},
91+
() => [this.count]
92+
);
93+
}
94+
95+
let foo = new Test();
96+
97+
assert.equal(foo.data.value, undefined);
98+
99+
foo.data.value;
100+
await settled();
101+
assert.equal(foo.data.value, 1);
102+
103+
foo.count = 2;
104+
foo.data.value;
105+
await settled();
106+
107+
assert.equal(foo.data.value, 2);
108+
109+
foo.count = 6;
110+
foo.data.value;
111+
await settled();
112+
113+
assert.equal(foo.data.value, 12);
114+
});
115+
});
116+
117+
module('in templates', function (hooks) {
118+
setupRenderingTest(hooks);
119+
120+
test('it works', async function (assert) {
121+
class Test extends Component {
122+
@tracked count = 1;
123+
124+
data = useFunction(
125+
this,
126+
(previous: number | undefined, count: number) => {
127+
return (previous || 1) * count;
128+
},
129+
() => [this.count]
130+
);
131+
increment = () => this.count++;
132+
}
133+
134+
const TestComponent = setComponentTemplate(
135+
hbs`
136+
<out>{{this.data.value}}</out>
137+
<button type='button' {{on 'click' this.increment}}></button>`,
138+
Test
139+
);
140+
141+
this.setProperties({ TestComponent });
142+
143+
await render(hbs`<this.TestComponent />`);
144+
145+
assert.dom('out').hasText('1');
146+
147+
await click('button');
148+
149+
assert.dom('out').hasText('2');
150+
});
151+
152+
test('async functions update when the promise resolves', async function (assert) {
153+
class Test extends Component {
154+
@tracked multiplier = 1;
155+
156+
increment = () => this.multiplier++;
157+
158+
data = useFunction(
159+
this,
160+
async (_, multiplier) => {
161+
// tracked data consumed here directly does not entangle with the function (deliberately)
162+
// let { multiplier } = this;
163+
164+
await new Promise((resolve) => setTimeout(resolve, 50));
165+
166+
return 2 * multiplier;
167+
},
168+
() => [this.multiplier]
169+
);
170+
}
171+
172+
const TestComponent = setComponentTemplate(
173+
hbs`
174+
<out>{{this.data.value}}</out>
175+
<button type='button' {{on 'click' this.increment}}></button>
176+
`,
177+
Test
178+
);
179+
180+
this.setProperties({ TestComponent });
181+
182+
render(hbs`<this.TestComponent />`);
183+
184+
await timeout(25);
185+
assert.dom('out').hasText('');
186+
187+
await settled();
188+
assert.dom('out').hasText('2');
189+
190+
click('button');
191+
await timeout(25);
192+
assert.dom('out').hasText('2');
193+
194+
await settled();
195+
assert.dom('out').hasText('4');
196+
});
197+
198+
test('it works without a thunk', async function (assert) {
199+
class Test extends Component {
200+
@tracked count = 1;
201+
202+
doubled = useFunction(this, () => this.count * 2);
203+
increment = () => this.count++;
204+
}
205+
206+
const TestComponent = setComponentTemplate(
207+
hbs`
208+
<out>{{this.doubled.value}}</out>
209+
<button type='button' {{on 'click' this.increment}}></button>`,
210+
Test
211+
);
212+
213+
this.setProperties({ TestComponent });
214+
215+
await render(hbs`<this.TestComponent />`);
216+
217+
assert.dom('out').hasText('2');
218+
219+
await click('button');
220+
221+
assert.dom('out').hasText('4');
222+
});
223+
});
224+
});

0 commit comments

Comments
 (0)