-
Notifications
You must be signed in to change notification settings - Fork 37
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import { tracked } from '@glimmer/tracking'; | ||
import { assert } from '@ember/debug'; | ||
import { action } from '@ember/object'; | ||
import { waitForPromise } from '@ember/test-waiters'; | ||
|
||
import { resource } from '../core/function-based'; | ||
|
@@ -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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
}); | ||
} | ||
|
||
(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() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why "execute" for the name? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also not happy with it but |
||
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() { | ||
|
@@ -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 | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 }]); | ||
}); | ||
}); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 😓