Allowing for asynchronous actions in afterSet
and delegating loadData
responsibility to setFilter
#22
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request is in response to #20.
In getting a subset of features implemented in the MBEE application, we found that
beforeSet
,afterSet
, andloadData
were all simultaneously firing - asynchronous behavior such asfetch
requests were not properlyawaited
.Context
In Harness applications, it is generally expected that setting a filter runs the
loadData
function. This is actually not a convention inharness-vue
, but a convention inharness-vue-bootstrap
, which are typically used in combination. All form control components inharness-vue-bootstrap
use the sameboundValue
(link) computed function as a getter/setter in a two-way bind using Vue's v-model. ThisboundValue
implementation checks for asynchronous
prop, aharness-vue-bootstrap
convention, and runsloadData
.harness-vue
providesbeforeSet
andafterSet
hooks to all filters, which use pinia's action susbcriptions to run before/after thesetFilter
action for a specific filter fires.Problem
The initial discovery of an issue was for the MBEE project. In this project, the filter options lived on the server side, so there were frequent
fetch
requests being triggered inbeforeSet
andafterSet
hooks. For example, setting filter A might trigger a fetch request to the server which is used to set the options and new default value for filter B.What we noticed was that none of these fetch requests were properly
await
ed -beforeSet
,loadData
andafterSet
were seemingly triggered simultaneously and resolved independently.The problem here was twofold - first, none of the
harness-vue
action subscription implementation was asynchronous. Second, theboundValue
computed setter inharness-vue-bootstrap
runssetFilter
andloadData
independently. Because of the design of the pinia subscriptions, there is no way to have thesetFilter
function wait for anyafter
effects to resolve as part of its own promise resolution.Solution
The solution to the first problem was easy(ish) - we revised the implementation of
setFilter
andloadData
to be asynchronous, as well as the subscription implementation. What we found in doing so is that pinia's action subscriptions handleafter
effects asynchronously, but notbefore
effects. This seems to be intended, and means that in our implementation, we can expect thatafter
effects are properly asynchronous, butbefore
effects are not - which is fine and is a limitation we can document.However, given how these things resolve independently, there was not a way to have
harness-vue-bootstrap
wait for theafterSet
's promise resolution before runningloadData
.Our solution is instead to bring this behavior into
harness-vue
. In this implementation we have changed the API so thatsynchronous
is no longer a prop defined byharness-vue-bootstrap
, but is instead now an attribute onharness-vue
filters calledtriggerLoadData
that defaults tofalse
. This allows us to move theloadData
call into the action subscription, so that we can properly expect a linear progression fromsetFilter
->afterSet
->loadData
.In doing so, we realized that sometimes we call
setFilter
inloadData
orafterSet
functions, but don't necessarily want to spawn a duplicateloadData
call. To alleviate this, we added an optional parameter tosetFilter
(defaulting totrue
) that tellsharness-vue
whether or not to callloadData
after the resolution ofsetFilter
andafterSet
.To summarize the proposed API changes:
loadData
is now a direct result ofsetFilter
setFilter
can now be called assetFilter(key, payload, triggerLoadData = true)
triggerLoadData: false
if you would never like them to triggerloadData
as a side effectI'm pausing here to check with the team in a call on Monday as to what ramifications this may have on existing codebases. I have a few thoughts that I'll capture there - specifically around the
triggerLoadData
parameter ofsetFilter
.