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

re-callable trackedFunction #559

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 50 additions & 28 deletions ember-resources/src/util/function.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { tracked } from '@glimmer/tracking';
Copy link
Owner

Choose a reason for hiding this comment

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

why so many formatting changes here? can you revert these? makes the diff a bit noisy

Copy link
Author

Choose a reason for hiding this comment

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

Done, prettier format on save 😓

import { assert } from '@ember/debug';
import { action } from '@ember/object';
import { waitForPromise } from '@ember/test-waiters';

import { resource } from '../core/function-based';
Expand Down Expand Up @@ -92,43 +93,53 @@ export function trackedFunction<Return>(...passedArgs: UseFunctionArgs<Return>)
fn = passedArgs[2];
}

return resource<State<Return>>(context, (hooks) => {
let state = new State(initialValue);
return resource<TrackedFunctionProperty<Return>>(context, (hooks) => {
return new TrackedFunctionProperty(fn, hooks, initialValue);
Copy link
Owner

Choose a reason for hiding this comment

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

Changing the return type is a bit of a breaking change -- any chance you can combine the two classes, in in the execute method, "reset" the state values?

});
}

(async () => {
try {
let notQuiteValue = fn(hooks);
let promise = Promise.resolve(notQuiteValue);
export class TrackedFunctionProperty<Value> {
@tracked state: State<Value>;

waitForPromise(promise);
constructor(private fn: ResourceFn<Value>, private hooks: Hooks, initialValue?: Value) {
this.state = new State(initialValue);
this.getValue();
}

let result = await promise;
async getValue() {
try {
let notQuiteValue = this.fn(this.hooks);
let promise = Promise.resolve(notQuiteValue);

state.error = undefined;
state.resolvedValue = result;
} catch (e) {
state.error = e;
} finally {
state.isResolved = true;
}
})();
waitForPromise(promise);

return state;
});
}
let result = await promise;

/**
* State container that represents the asynchrony of a `trackedFunction`
*/
export class State<Value> {
@tracked isResolved = false;
@tracked resolvedValue?: Value;
@tracked error?: unknown;
this.state.error = undefined;
this.state.resolvedValue = result;
} catch (e) {
this.state.error = e;
} finally {
this.state.isResolved = true;
}
}

constructor(public initialValue?: Value) {}
@action
execute() {
Copy link
Owner

Choose a reason for hiding this comment

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

why "execute" for the name?

Copy link
Author

Choose a reason for hiding this comment

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

I'm also not happy with it but perform is no kind of reserved for tasks, run, call etc also seem a bit off and retry is imo trying something that has failed again. Whats your opinion?

this.state = new State(this.state.initialValue);
this.getValue();

return this.state;
}
get isResolved() {
return this.state.isResolved;
}
get error() {
return this.state.error;
}

get value() {
return this.resolvedValue || this.initialValue || null;
return this.state.resolvedValue || this.state.initialValue || null;
}

get isPending() {
Expand All @@ -144,6 +155,17 @@ export class State<Value> {
}
}

/**
* State container that represents the asynchrony of a `trackedFunction`
*/
export class State<Value> {
@tracked isResolved = false;
@tracked resolvedValue?: Value;
@tracked error?: unknown;

constructor(public initialValue?: Value) {}
}

/**
* @private
*
Expand Down
43 changes: 43 additions & 0 deletions testing/ember-app/tests/utils/function/js-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,47 @@ module('Utils | trackedFunction | js', function (hooks) {

assert.strictEqual(foo.data.value, 2);
});

test('it can be reexecuted', async function (assert) {
const Store = {
data: [[{ id: 1 }], [{ id: 1 }, { id: 2 }], [{ id: 1 }], [{ id: 1 }]],
fetch(page: number) {
const value = this.data.shift();

return value?.map((v) => ({ ...v, page }));
},
};

class Test {
store = Store;
@tracked page = 1;

@tracked data = trackedFunction(this, () => {
return this.store.fetch(this.page);
});
}

let foo = new Test();

foo.data.value;
await settled();
assert.deepEqual(foo.data.value, [{ id: 1, page: 1 }]);

foo.page = 2;
foo.data.value;
await settled();
assert.deepEqual(foo.data.value, [
{ id: 1, page: 2 },
{ id: 2, page: 2 },
]);

const value = await foo.data.execute();
Copy link
Owner

Choose a reason for hiding this comment

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

can you add assertions for what the value properties are supposed to be after execute is invoked?

maybe something like:

let almostValue = foo.data.execute();
// assert stuff
assert...

let value = await almostValue


await settled();
assert.deepEqual(value.resolvedValue, [{ id: 1, page: 2 }]);

await foo.data.execute();
await settled();
assert.deepEqual(foo.data.value, [{ id: 1, page: 2 }]);
});
});