Open dphfox opened 3 years ago
This would enhance the current suboptimal semantics around yielding code right now - instead of having to resort to using a Compat to listen and send off yielding changes, it'd be nice to have some object that wraps a Promise in a state-object-like interface that always resolves to a value without yielding, so it can be used directly like a Computed or State object would be.
So I've been considering this for a while, and with the advent of explicit dynamic dependencies (see #111) it should now be possible to extend the idea of a Computed to support yielding. Since this doesn't depend specifically on using promises, I'm going to adapt this issue to cover async/yielding code more generally.
I see two paths forward. Either:
Both approaches look very similar, and are based around the idea of a 'pending state'. Since the only requirement preventing yielding is that all state objects are always consistent with each other, we could change the behaviour of Computed from 'disallowing yielding to ensure that the value always reflects the true values of other state objects' to 'while yielding, the value of the computed is made inaccessible, with some way of detecting the pending-ness of the object'.
To demonstrate what I mean, let me hand-wave a new object, Eventual
, into existence. This could end up being the object we implement, or alternatively, Computed
could absorb these ideas.
One possibility is returning a well-defined value in place of the calculated value during yielding:
local number = Value(5)
local plusOne = Eventual(function(use)
task.wait(5) -- this is really difficult to compute! takes a lot of time!
return use(number) + 1
end)
print(plusOne:get()) --> "pending"
task.wait(5)
print(plusOne:get() --> 6
In this example, it's just the string "pending", but in the real world this would probably be a symbol exposed via Fusion. The simplicity of this approach is appealing at first glance.
An alternate design could separate out the pending state and the computed state - :get()
would now error if you attempt to use a value before it's calculated;
local number = Value(5)
local plusOne = Eventual(function(use)
task.wait(5)
return use(number) + 1
end)
-- plusOne:get() would error if we called it here
print(plusOne:isPending()) --> true
task.wait(5)
print(plusOne:isPending()) --> false
print(plusOne:get()) --> 6
One thing I like about this alternate design is that it strongly guarantees an error when you use a value that isn't ready - something which is almost always error-worthy, at least in all the cases I can think of. Strong and well-defined error conditions are good here. We could provide a further separate method, :await()
, to explicitly yield until the eventual resolves.
I would be intrigued to hear what you guys think of this. It could also be interesting to explore how this stuff will interact with instance trees - in particular, React has the concept of Suspense for declaratively defining loading UI, which we could possibly borrow?
I wouldn't like Fusion to start depending on promises due to the fact that I've never really used promises in my code, so a more general solution would definitely be desired on my end. Looking forward to seeing where this goes :)
After further discussion in the OSS discord, leaning towards rolling the former solution into Computed using a symbol:
local number = Value(5)
local plusOne = Computed(function(use)
task.wait(5) -- this is really difficult to compute! takes a lot of time!
return use(number) + 1
end)
print(plusOne:get()) --> { type = "Symbol", kind = "Pending" }
task.wait(5)
print(plusOne:get() --> 6
I should probably drop these thoughts here from the Discord:
yeah I'm always searching for the most theoretically fitting answer, and the first solution lines up more with how Fusion works in my mind State objects store state Pending is a state just like any resolved value is a state Therefore, state objects should be able to store pending The presence of a pending state leads to graph consistency Which means Lua-style yielding can be accommodated without introducing disagreements between variables All that without adding to the API surface or standard interface for state objects The second solution can achieve this too, but it does so while introducing currently-foreign concepts such as multi-valued state objects, possibly adding an error condition to :get(), and promoting the use of yielding to signify pending state rather than making that declarative Multi-valued state objects are something that have come up before in the context of follower objects, specifically for allowing user code to monitor sleeping, but there's no clear answer for whether multi-valued state objects are even the correct answer yet
One point of contention that has come up here is about how the resolution status of an Eventual
or Computed
is to be represented. There are a few philosophies here:
Eventual<T>:get()
returns Pending | Resolved<T>
(analogous to an Option<T>
monad)Eventual<T>:get()
returns Pending | T
(simple idiomatic Luau)Eventual<T>:get()
returns T
and errors if pending, pending status returned by Eventual<T>:isPending()
o.e.The first two approaches roughly align with solution 1, while the last approach roughly aligns with solution 2 (though sans :await()
). I've been constantly changing my mind ever since I put both solution forward as to which one is actually better, and it's not exactly something we can easily prototype either because we're trying to theorise how this stuff plays out at large scale in real codebases. We will likely end up trialling multiple implementations of this object.
One thing to note about the difference between monads and union typing: though they're broadly isomorphic, the key difference between them is that the monad wraps the return value in a table, meaning unwrapping becomes mandatory, while union typing does not wrap the return value. The advantage of wrapping the value means that it is incompatible by default with any other logic not dealing with Eventual<T>
, meaning the pending state must either be handled or discarded somewhere. Those 'unwrapping' points then become the only places where errors can arise from not properly handling the absence of a value. The downside is that it is incompatible by default with any other logic not dealing with Eventual<T>
- this complicates some things such as using an eventual value in an instance property. This might however end up being desirable because is the concept of an instance property being Pending even a sensible notion?
One interesting line of thinking here - should an Eventual
be able to control it's value during yielding? Perhaps, instead of providing a Pending
symbol/monad/equivalent, it returns one value for each non-yielding part of the function:
-- psuedocode - this is not valid Lua
local something = Eventual(function()
return "Loading..."
task.wait(5)
return 42
end)
I'll refer to this from now on as solution 3. We could probably implement this with coroutines somehow.
I think I've changed my mind on the idea of consolidating Eventual
with Computed
. I think there is utility in cleaning separating yielding code away from non-yielding code in some way, as this serves to indicate what values and behaviour the user of these objects should expect, and helps to catch bugs when someone accidentally yields in a Computed.
A variant on solution 3 is by far the most promising to me. I think this is almost a perfect solution for the design requirements of both Fusion and this issue!
The basic idea is pretty simple - give the Eventual a value to use while your function runs:
-- When computation starts, the object switches to the "Loading..." value
-- Once the computation returns, the object switches to the final value
local thing = Eventual("Loading...", function(use)
task.wait(5)
return "Hello, world!"
end)
This should be immediately intuitive to basically anyone who knows how a Computed works, and is completely uncomplicated - no advanced features, algorithms or extra concepts needed to understand it. Plus, it doesn't depend on the use of a custom symbol to indicate pending status; the user still has the option of providing one, of course, but since most people just want to show fallback content, it's pragmatic to provide this utility directly as standard.
The best part is that we can incorporate the coroutine-like yielding from solution 3 to make it trivial to implement more complex kinds of transition:
local thing = Eventual("Loading...", function(use, become)
for i=10, 1, -1 do
task.wait(1)
become("Returning in " .. i .. " seconds")
end
return "Hello, world!"
end)
This will be absolutely indispensable for anyone who needs to show a progress bar - as the long-running task progresses, the Eventual can update itself via become
to reflect the percent completion of the task.
Of course, this simplicity of implementation has many knock on benefits. Monads are 100% natural to use:
local thing = Eventual(Option.None, function(use)
return Option.Some(ReplicatedStorage.GetScore:InvokeServer())
end)
Promises are a doddle:
local thing = Eventual("Loading...", function(use)
local _, value = assert(myPromise:await())
return value
end)
You can even use it inline just like a Computed - a huge ergonomic win:
local avatar = New "ImageLabel" {
Image = Eventual(GENERIC_AVATAR, function(use)
return Players:GetUserThumbnailAsync(
use(userId),
Enum.ThumbnailType.HeadShot,
Enum.ThumbnailSize.Size100x100
)
end)
}
In general, this really feels like the right solution.
The best part is that we can incorporate the coroutine-like yielding from solution 3 to make it trivial to implement more complex kinds of transition:
local thing = Eventual("Loading...", function(use, become) for i=10, 1, -1 do task.wait(1) become("Returning in " .. i .. " seconds") end return "Hello, world!" end)
This will be absolutely indispensable for anyone who needs to show a progress bar - as the long-running task progresses, the Eventual can update itself via
become
to reflect the percent completion of the task.
I think it signals intent more clearly to put an explicit become(firstValue)
, removing the first parameter from Eventual, making it the same signature as Computed. At that point, is it still necessary to have the distinction at all?
Doing this also allows a first value that is dependent on other reactive graph items.
If that approach is taken, what would be returned if the callback does not become
before yielding? It could be a symbol, Fusion.Pending, like solution 1, but I would personally say the Eventual should error on the first yield if no become
has been set yet.
How do dependencies work in an Eventual? They could work by returning their state when the Eventual first started processing. Or they could return their current value. However, the latter sounds like it could very easily make race conditions in user code. If using the former, how does Eventual work when its dependencies update while still/already processing? Should a new thread be launched immediately, and its become
values used? If so, what happens to the old thread? We could use coroutine.close
, but this may lead to issues with garbage collection if things are initialised earlier on in the callback? I haven't yet found any solution here that covers all the bases.
I think it signals intent more clearly to put an explicit
become(firstValue)
, removing the first parameter from Eventual...
The downside here is that calling become()
is an optional action at 'compile' time - you don't have to include it. However, if you don't, you get an error at runtime. It would be more ideal for the prevention of common errors to have the mandatory initial value be passed in a way which can't be skipped over by accident - for example, by making it the first argument passed to Eventual. The logic here is mostly about error prevention.
...making it the same signature as Computed. At that point, is it still necessary to have the distinction at all?
Beyond the API similarities, it's important to consider user intent. Yielding can easily be accidentally introduced into a system in Roblox, which can cause unintuitive and hard to track down bugs. Furthermore the distinction might change the considerations for users of the value (i.e. if you're using an Eventual, you will probably be thinking about how to handle the unresolved state, whereas a Computed does not prime the user for thinking about this explicitly). For those reasons I think that it is worth keeping yielding separate from Computed, regardless of what we do API wise.
Doing this also allows a first value that is dependent on other reactive graph items.
This is a fair point.
If that approach is taken, what would be returned if the callback does not
become
before yielding? It could be a symbol, Fusion.Pending, like solution 1, but I would personally say the Eventual should error on the first yield if nobecome
has been set yet.
I would agree on an error here - we should try not to implicitly fill things in for the user in most cases. For example, if a developer forgets to call become
, it is more useful to flag that up so they can intentionally choose a good value, as opposed to filling it in with a general and barely contextually meaningful value.
How do dependencies work in an Eventual? They could work by returning their state when the Eventual first started processing. Or they could return their current value. However, the latter sounds like it could very easily make race conditions in user code. If using the former, how does Eventual work when its dependencies update while still/already processing? Should a new thread be launched immediately, and its
become
values used? If so, what happens to the old thread? We could usecoroutine.close
, but this may lead to issues with garbage collection if things are initialised earlier on in the callback? I haven't yet found any solution here that covers all the bases.
A simple solution is to force all dependencies to be retrieved before the first yield, and error if the callback attempts to add a dependency thereafter.
If a dependency changes duing evaluation, it should be fine to terminate early and recalculate from scratch. Eventual is still expected to play by the same 'semi-pure' rules as Computed does, and as such there should not be consequences for terminating its execution early if it must recalculate. Idiomatic Fusion code does not have stateful Computed callbacks, and so stateful Eventual callbacks would also not be allowed, which should eliminate any problems that could arise from early termination.
I don't believe there's any technical reason why use couldn't be called after the first yield, right? It's just that it retrieves the value at a different point in time. Perhaps instead of emitting an error here, a warning could suffice.
I think the design for this is sufficiently agreed upon that it can be safely solidified save for a few nuances.
I don't believe there's any technical reason why use couldn't be called after the first yield, right? It's just that it retrieves the value at a different point in time. Perhaps instead of emitting an error here, a warning could suffice.
Race conditions. I think it's a sign of bad practice if the Eventual result depends on state A
at [1s after game start] but also on state B
at [2s after game start].
I think these two snippets should behave the same:
local thing = Eventual(function(use, become)
become("Loading...")
local a = use(A)
wait(1)
local b = use(B)
become(a..b)
end)
local thing = Eventual(function(use, become)
become("Loading...")
local a = use(A)
local b = use(B)
wait(1)
become(a..b)
end)
But if B
updates during the wait, the behaviour is different.
I don't think a warning makes sense. You'd be spamming the output if the developer wants to do the bad practice; if they don't, then the warning might be subtle and easily missed.
Should the final result be the last become
, or be the callback's return
value? I'd argue the former, because it allows ending the execution with return
if it turns out the latest become
was already correct. For example, if fetching multiple Pages
from a Roblox API, ending with return
if the next page is empty.
Race conditions. I think it's a sign of bad practice if the Eventual result depends on state A at [1s after game start] but also on state B at [2s after game start].
Note that the processor function is cancelled if any dependency changes during execution, which should include any dependencies added before yielding. Given this, imagine that the value of B changes during that one second wait. If B was used before, the Eventual restarts processing and so will use the correct value. If B was used after, the Eventual will see the correct value when it gets there. The only difference is probably how fast you'll get to the right result. Is the correctness of the value what you are referring to when you say 'race condition' or is it more broadly about executing work in the most efficient way?
I don't think a warning makes sense. You'd be spamming the output if the developer wants to do the bad practice; if they don't, then the warning might be subtle and easily missed.
Yeah agreed.
Should the final result be the last become, or be the callback's return value? I'd argue the former, because it allows ending the execution with return if it turns out the latest become was already correct. For example, if fetching multiple Pages from a Roblox API, ending with return if the next page is empty.
That does indeed seem useful. My only reservation is that it would make simpler usages of Eventual feel different from Computed, but perhaps that doesn't matter.
fwiw, Eventual seems (at least from the perspective of an external user) to be equivalent to a Computed connected to a state machine. The states of the machine represent the different states the Eventual can be in when it has yielded/returned, and the Computed object then switches on this state to determine what dependencies to use and what value to return. Obviously internally this isn't how things work, but it should be relatively equivalent from the perspective of the reactive graph or of a downstream user. So I think this design of Eventual should preserve all of Fusion's existing guarantees about the reactive graph.
Race conditions. I think it's a sign of bad practice if the Eventual result depends on state A at [1s after game start] but also on state B at [2s after game start].
Note that the processor function is cancelled if any dependency changes during execution, which should include any dependencies added before yielding. Given this, imagine that the value of B changes during that one second wait. If B was used before, the Eventual restarts processing and so will use the correct value. If B was used after, the Eventual will see the correct value when it gets there. The only difference is probably how fast you'll get to the right result. Is the correctness of the value what you are referring to when you say 'race condition' or is it more broadly about executing work in the most efficient way?
I was under the impression that the callback would be allowed to finish, and start a second run at the same time if a dependency changed. But since that's not the case, nevermind my original logic about race conditions.
I'm unsure about the restarting behaviour. It seems like it might be easy to accidentally make things take long with no result in the meantime. For example, if A is a Spring or Tween. Then again, using a gradual State doesn't really make sense for an Eventual anyways, and I can't think of a better catch-all solution to cover the behaviour.
I was under the impression that the callback would be allowed to finish, and start a second run at the same time if a dependency changed. But since that's not the case, nevermind my original logic about race conditions.
I'm unsure about the restarting behaviour. It seems like it might be easy to accidentally make things take long with no result in the meantime. For example, if A is a Spring or Tween. Then again, using a gradual State doesn't really make sense for an Eventual anyways, and I can't think of a better catch-all solution to cover the behaviour.
Yeah thats my logic atm. Better to ensure correctness eventually than to output results that aren't consistent with the rest of the reactive graph. Inconsistency causes a lot of confusion.
Open question: in a world where lazy execution is implemented (#144) how will Eventual be evaluated? Should it be eagerly evaluated, so that it starts processing as soon as it is notified of a change (forcing all state objects upstream to evaluate)? Or should it be lazily evaluated, starting work only when it is asked to, which is more efficient overall but possibly introduces delays?
I suggest a destructor parameter for Eventual.
I suggest a destructor parameter for Eventual.
Eventual will adopt whatever convention is used by Computed et al for consistency. This is open to change with the work being done in #269
Here's a question: how should Eventual
handle a callback error? My take is that it should fall back to a default value, and explicitly should not preserve any value provided by any become()
calls (because those may be temporary or destroyed after the callback terminates).
This recontextualises the first parameter as a "not available" rather than a "loading" state; to distinguish loading and error states, you'd have to become()
the loading state at the start of the callback, and store the error state as the default.
Alternatively, it could fall back to the last fully resolved value, like a Computed does.
Alright, let me pin down a few of these nuances:
I don't believe there's any technical reason why use couldn't be called after the first yield, right? It's just that it retrieves the value at a different point in time. Perhaps instead of emitting an error here, a warning could suffice.
A warning is not necessary. If you have not used a value yet, then it has no way of influencing the computation thus far. Therefore, there is no universe in which a computation would be invalidated by a value being changed between the computation starting and the computation using the value, because the value cannot influence the computation in that time.
For that reason, it's actually desirable to leave use()
calls as late as possible, because this minimises the chance of a computation needing to restart. So, Fusion won't warn for this, unless there's something actually show-stopping in the implementation (but I'm reasonably confident it's possible to do).
Here's a question: how should Eventual handle a callback error?
The same way a Computed does - revert to the last value. This is best for consistency; we can address whether we should have a better error-handling behaviour some other time, as part of a broader review of error handling in Fusion.
Open question: in a world where lazy execution is implemented (https://github.com/dphfox/Fusion/issues/144) how will Eventual be evaluated?
Lazily. If eager behaviour is needed, it can always be added by an Eager
object, per that issue's suggested API surface. Lazy execution is consistent with the rest of Fusion, trims down on unused work, and gives the developer more flexibility when tuning the performance of their program.
Should the final result be the last become, or be the callback's return value? I'd argue the former, because it allows ending the execution with return if it turns out the latest become was already correct. For example, if fetching multiple Pages from a Roblox API, ending with return if the next page is empty.
The callback's return value. Again, this is consistent with Computed objects, which simplifies the learning curve of this object type. This trades trivially reusing the last become
, with trivially early returning. No matter what we choose, going with one will make the other ever-so-slightly harder. I foresee that early returns will be more common, so to facilitate that, return values will overwrite become
values.
I considered only using the last become
when returning nil
(or - more advanced - returning zero values) but this would prove awkward when nil
is a significant return value, or otherwise would prove unintuitive and become a common gotcha.
For that last bit - would early returns not be easy using return become(finalValue)
? I feel like the code overhead for having to store the most recent become
value (at least in some cases) outweighs that. Then again, it's not much of a deal breaker.
There really isn't much difference either way, so I decided to keep the shape of early returns similar to Computed.
There shouldn't really be any notable overhead from creating an extra variable, and it perhaps makes the intention of the programmer more explicit when you want to reuse values? It's not all that different though.
It could be nice for Fusion to better support promises as part of its core functionality - for example, being able to conditionally render a loading UI, result UI or error UI based on whether a promise is unresolved, resolved or rejected.
I've only personally used promises outside of Roblox, and so don't yet have enough experience with promises in Lua to reason about what API designs would be most suitable for better supporting them in Fusion (if any supporting API is needed).