Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some timing tests for trackedFunction #1011

Merged
merged 8 commits into from
Oct 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/neat-apples-shout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"ember-resources": patch
---

`trackedFunction`: Fix timing issue where updating tracked data consumed in `trackedFunction` would not re-cause the `isLoading` state to become `true` again.

Resolves #1010
7 changes: 7 additions & 0 deletions ember-resources/src/util/function.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,13 @@ export class State<Value> {
retry = async () => {
if (isDestroyed(this) || isDestroying(this)) return;

// We've previously had data, but we're about to run-again.
// we need to do this again so `isLoading` goes back to `true` when re-running.
// NOTE: we want to do this _even_ if this.data is already null.
// it's all in the same tracking frame and the important thing is taht
// we can't *read* data here.
this.data = null;

// We need to invoke this before going async so that tracked properties are consumed (entangled with) synchronously
this.promise = this.#fn();

Expand Down
2 changes: 1 addition & 1 deletion test-app/tests/utils/function/rendering-test.gts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';
import { setOwner } from '@ember/application';

import { use, resource, resourceFactory } from 'ember-resources';
import { cell, use, resource, resourceFactory } from 'ember-resources';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this cell import is unused, probably leftover from investigation

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, yeah, I need to fix linting on this repo 🙃
there is a bug with prettier rn though, and I'm waiting for them to do another release

import { trackedFunction } from 'ember-resources/util/function';

const timeout = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms));
Expand Down
111 changes: 111 additions & 0 deletions test-app/tests/utils/function/timing-test.gts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import Component from '@glimmer/component';
import { concat } from '@ember/helper';
import { render, settled } from '@ember/test-helpers';
import { module, test } from 'qunit';
import { setupRenderingTest } from 'ember-qunit';

import { cell, resource, resourceFactory } from 'ember-resources';
import { trackedFunction } from 'ember-resources/util/function';

module('Utils | trackedFunction | timing', function (hooks) {
setupRenderingTest(hooks);

test('With Argument', async function (assert) {
let step = (msg: string) => assert.step(msg);

let state = cell(0);

async function fn(value: number) {
step(`fn:begin:${value}`);
await Promise.resolve();
step(`fn:end:${value}`);
return `yay:${value}`;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😄

}

const WithArgument = resourceFactory(num => resource(({ use }) => {
let reactive = use(trackedFunction(() => fn(num)));

// TODO: the types should allow us to directly return the use,
// but they don't currently
return () => reactive.current;
}));

await render(
<template>
{{#let (WithArgument state.current) as |request|}}
{{#if request.isLoading}}
{{step 'loading'}}
{{/if}}
{{#if request.isError}}
{{step 'error'}}
{{/if}}
{{#if request.value}}
{{step (concat 'loaded:' request.value)}}
{{/if}}
{{/let}}
</template>
);

assert.verifySteps([
'fn:begin:0', 'loading', 'fn:end:0', 'loaded:yay:0'
]);

state.current = 1;
await settled();

assert.verifySteps([
'fn:begin:1', 'loading', 'fn:end:1', 'loaded:yay:1'
]);
});

test('From a component class', async function (assert) {
let step = (msg: string) => assert.step(msg);

let state = cell(0);

class Example extends Component<{ Args: { value: unknown } }> {
request = trackedFunction(this, async () => {
let value = this.args.value;
step(`fn:begin:${value}`);
await Promise.resolve();
step(`fn:end:${value}`);
return `yay:${value}`;
});

<template>
{{#if this.request.isPending}}
{{step 'pending'}}
{{/if}}
{{#if this.request.isLoading}}
{{step 'loading'}}
{{/if}}
{{#if this.request.isError}}
{{step 'error'}}
{{/if}}
{{#if this.request.value}}
{{step (concat 'loaded:' this.request.value)}}
{{/if}}
{{#if this.request.isFinished}}
{{step 'finished'}}
{{/if}}
</template>
}

await render(
<template>
<Example @value={{state.current}} />
</template>
);

assert.verifySteps([
'fn:begin:0', 'pending', 'loading', 'fn:end:0', 'loaded:yay:0', 'finished'
]);

state.current = 1;
await settled();

assert.verifySteps([
'fn:begin:1', 'pending', 'loading', 'fn:end:1', 'loaded:yay:1', 'finished'
]);
});
});