search

square / flow

Name UI states, navigate between them, remember where you've been.
Apache License 2.0
2.79k stars 241 forks source link

New way to break Flow.goBack() #264

Open dcow opened 7 years ago

dcow commented 7 years ago

I have encountered an issue where the history size shrinks while a traversal is in-flight. This could happen any time you hold traversals while you're switching screens (perhaps for an animation) and mash the back button quickly. It appears the issue is noted:

https://github.com/square/flow/commit/8a9325bc89118d3f6704e4e65f13231bf4efc407#diff-0351f5577272525804cf0292acb7fe1bR283

195 #240

but I don't think the solution of simply noop-ing the traversal is entirely correct. When this happens, the dispatcher never gets to execute the next pending traversal. It may be a good idea to noop the traversal, but the dispatcher still needs to flush traversals so that the whole thing doesn't grind to a halt. Another option might be to continue dispatching the traversals but not pop the history if Flow notices the history has changed size.

A workaround is to not hold onto traversals, but that's not really what you want to do long term.

Zhuinden commented 7 years ago

So I'm guessing the problem is that it does a return; and not an immediate onTraversalCompleted() (which wouldn't work because it's not technically "dispatched"), and therefore somehow you can enqueue ops after the enqueued goBack() but it is not started because of return; instead of starting the next enqueued traversal if possible?

https://github.com/square/flow/blob/2982c5eb26fdc2b93e1d9b20f242c062ad73edb5/flow/src/main/java/flow/Flow.java#L331-L341

Zhuinden commented 7 years ago

Have you figured out if my hunch was right or wrong? I mean, I haven't used Flow in a while.

sdelaysam commented 7 years ago

@dcow here how I solved the issue for me https://github.com/sdelaysam/flow/commit/4980cefd298fc33068fa155801f6423b89a0a57f