[WIP] Update FlowManager in Managed Player state if a new one is passed in #585
Conversation
KetanReddy
added
bug
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #585 +/- ##
==========================================
+ Coverage 89.76% 89.77% +0.01%
==========================================
Files 331 331
Lines 19840 19863 +23
Branches 1949 1951 +2
==========================================
+ Hits 17809 17832 +23
Misses 2017 2017
Partials 14 14 ☔ View full report in Codecov by Sentry. |
|
/canary |
| React.useEffect(() => { | ||
| if (managedState.state) { | ||
| managedState.state.context.manager = props.manager; | ||
| } | ||
| }, [props.manager]); |
There was a problem hiding this comment.
IMO this feels like we should be terminating the old managed-player before picking up with the new one?
There was a problem hiding this comment.
Yeah this was just me hacking things together to test if this was the source of the issue. In theory a new Flow Manager should only be passed when managed player is in a completed state before the pending state builds the next function called later in the pending state. If the parent app wants to override the initial Flow Manager it started player with, it should only be able to do so between flows.
| options.middleware, | ||
| ); | ||
|
|
||
| const [managedState, setManagedState] = React.useState(initialManagedState); |
There was a problem hiding this comment.
Do we need this kept in state, or could it be a ref instead?
There was a problem hiding this comment.
Since it should trigger a re-render when we create a new managed state I figured it should be kept in state. I guess technically because things will re-render anyways when the new flow comes in we don't need to do that twice. I can move it over to a ref.
|
/canary |
If a new
FlowManagerinstance is passed in while aMangedPlayerinstance is running, when managed player transitions it will be using the old flow manager instance.Change Type (required)
Indicate the type of change your pull request is:
patchminormajorN/ADoes your PR have any documentation updates?
Release Notes
TBD
📦 Published PR as canary version:
0.10.3--canary.585.19781Try this version out locally by upgrading relevant packages to 0.10.3--canary.585.19781