xstate
xstate copied to clipboard
statelyai
Bug: Overeager evaluation of entry and exit actions
Description
It seems entry and exit actions are fired repeatedly when the state is entered/exited. I stumbled upon the bug somewhat accidentally and noticed an entry action running 103 times in my project when the machine started up. Changing it to an exit action resolved the issue and was suitable for my use case but in other places, exit actions are also running multiple times. The number of times it runs seems to be strangely predictable in the same machine but different across machines. For example, my issue above seeing the action run 103 times happens consistently with the same number of logs and in another machine, it happens 6 times. Both are reproducible but with differing counts. I put together a small codesandbox that illustrates the issue. You'll notice the serialized action runs 4 times and the inline action runs twice.
Expected result
Actions should only run once unless an explicit event invokes them again.
Actual result
Actions are running multiple times in a single execution chain.
Reproduction
https://codesandbox.io/s/little-cherry-hlnss2?file=/src/App.js
Additional context
No response
You are using React's StrictMode and thus the machine gets "restarted" because it starts in React's effect. You can read more about the changes introduced in StrictMode in React 18 here: https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#updates-to-strict-mode
@Andarist I forgot about that being the default in codesandbox and it would explain the issue there, but I have a machine that runs its entry action 103 times in my work project. I can't really post sample code from that in any useable way but I can try to repro it in codesandbox without StrictMode turned on. Turning that off halves the executions, which makes sense, but the assign still runs twice and runs a lot more in larger, more complex machines. This is a sample log from my actual app and when subscribing to the interpreter and logging the events that come through, the only one is { type: "xstate.init" }.
LOG in assign 1659914752180
LOG in assign 1659914752183
LOG in assign 1659914752186
... 97 more of the same ...
LOG in assign 1659914752778
LOG in assign 1659914752781
LOG in assign 1659914752784
You can see those assigns are running every few milliseconds until the state is exited.
The simplified machine that generates the above log is as follows. I've omitted a lot of states but those shouldn't affect this behavior as this state is a one-time state:
const machine = createMachine(
{
tsTypes: {} as import("./machine.typegen").Typegen0,
schema: {
context: {} as Context,
events: {} as Event,
services: {} as Services,
},
type: "parallel",
states: {
ui: {
initial: "waiting",
states: {
waiting: {
entry: "cache initial input",
always: "location",
},
location: {
on: {
next: "day",
},
},
},
},
},
},
{
actions: {
"cache initial input": assign(({ input }) => {
console.log("in assign", Date.now())
return { initialInput: input }
}),
},
},
)
The codesandbox I provided in the main post does exhibit the issue with StrictMode removed but it only evaluates the assign twice (which is still a problem, especially for something like telemetry or other fire-and-forget API calls). I've updated my codesandbox to hopefully be a bit more revealing. It now seems to me that the issue is in actions that use the Assigner overload of the action creators. You can see in the logs that the pure action and the assign action are executed three times while the non-action creator is executed once. I hope this enough to see there's a genuine bug and even these minor changes caused the entry count to go from 2 to 3 so you can imagine in larger machines/components that it could grow much larger (and does). Additionally, since the actions seem to run before the next "tick", context values are not updated multiple times as the assigns are being evaluated with the same value of context and not any value it may have been changed to through the series of erroneous calls. So it seems in a lot of cases, these extra executions won't be a huge problem, but as I mentioned, with any action that has side effects, those side effects will run multiple times and could cause problems.
I think I may have identified the source of the issue...it seems to be somewhat proportional to the number of useSelector calls (at least in how I'm using it). I added two repeated useSelector calls that select the same value and the number of executions of the entry action has increased accordingly.
Is it my use of useSelector that's incorrect? I was under the impression that I can select out whatever I want from the machine to reduce re-renders but it also seems to be the cause of this issue.
Sorry for all the comments...but this seems to be confirmed. By removing the calls to useSelector, the action runs once. With each added call to useSelector, an additional action is fired.
it only evaluates the assign twice (which is still a problem, especially for something like telemetry or other fire-and-forget API calls)
Keep in mind that assign actions should be pure.
@davidkpiano yea it occurred to me while typing that that in the case of telemetry and any fire-and-forget actions that hit APIs, they shouldn't be done inside of a pure or assign or any other action creator (and I don't abuse them that way). It still does seem to have performance implications though even if the end result is the same. For example, in my case where I had 103 entry actions being run, changing that to an exit, which in my case solved the issue, had noticeable performance improvements in the UI.
I've definitely tracked this down to be an issue with useSelector though. Am I using them incorrectly? In theory, I could create a single selector that returns a grouping of what all my current selectors return but that doesn't feel like a "best practice" to me. Reading the xstate docs, it seems my usage of useSelector is correct and that this is unintended behavior.
At the moment, entry assign actions for the initial state might get re-executed multiple times like this.
The current state of a not started interpreter is read like this within useSelector:
https://github.com/statelyai/xstate/blob/214baebba2f88317d7081ff13598a7a59422bc72/packages/xstate-react/src/utils.ts#L23
We can't read service.initialState (and cache it in the service) because it might lead to creating spawned actors which can't happen before React's commit phase. I think that we could cache some kind of service._pureInitialState though to remedy this a little bit. Note though that this would still execute those assign actions at least twice - once when computing this _pureInitialState and the second time when actually starting the machine in an effect.
This issue is mostly already fixed in v5 where we have refactored the implementation and we can cache the true initialState more easily.
As to the perf implications - I would love to see a profile of your app to check out how long does it take to compute/execute this stuff in your case and to investigate what we could do to improve the situation.
It seems this link is broken: https://codesandbox.io/s/little-cherry-hlnss2?file=/src/App.js
Closing as the original issue is unrelated to XState and more related to how React strict mode works, with double-effect execution. If the problem can be reproduced and pinpointed to useSelector, we can reopen this issue.