-
Notifications
You must be signed in to change notification settings - Fork 819
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Execute multiply nested SequenceMsg and BatchMsg in the proper order. #847
Comments
I am not sure this issue is a new feature addition. This might be a bug fix. |
How to reproduce the problemChange func (m model) Init() tea.Cmd {
return tea.Sequence(
tea.Batch(
tea.Sequence(
SleepPrintln("1-1-1", 1000),
SleepPrintln("1-1-2", 1000),
),
tea.Batch(
SleepPrintln("1-2-1", 1500),
SleepPrintln("1-2-2", 1250),
),
),
tea.Println("2"),
tea.Sequence(
tea.Batch(
SleepPrintln("3-1-1", 500),
SleepPrintln("3-1-2", 1000),
),
tea.Sequence(
SleepPrintln("3-2-1", 750),
SleepPrintln("3-2-2", 500),
),
),
tea.Quit,
)
}
// print string after stopping for a certain period of time
func SleepPrintln(s string, milisecond int) tea.Cmd {
printCmd := tea.Println(s)
return func() tea.Msg {
time.Sleep(time.Duration(milisecond) * time.Millisecond)
return printCmd()
}
} If you execute sequence with the current implementation of tea.go, you will get the following.
This is because However, with #848 the program works well.
|
So is this issue dead? just ran into this and spent a lot of time trying to figure out what was happening. And I was just using a nested Tried #848 and it worked fine for my case. |
Oh, I totally forgot this PR! Yes I think this should be solved. |
@meowgorithm Could you check this issue about |
Is your feature request related to a problem? Please describe.
Description
As stated in issue #680, the current
eventLoop
method handling forsequenceMsg
executes only once nestedBatchMsg
in the proper order, but nestedsequenceMsg
and multiple nestedBatchMsg
orsequenceMsg
will not follow the specified order.This is due to the fact that the
eventLoop
method callsp.Send
without checking the result of the message that is the result of the call to the command contained insequenceMsg
all the way through the nest.Example
The following shows the desired and actual calling order for a given
sequenceMsg
.These are important points of the example above.
tea.Batch
, so Command1-1 and Command1-2 are executed in parallel.tea.Sequence
, so Command1-1-1 and Command1-1-2 are executed in sequence.tea.Batch
, so Command1-2-1 and Command1-2-2 are executed in parallel.tea.Sequence
, so Command3-1 and Command3-2 are executed in sequence.tea.Batch
, so Command3-1-1 and Command3-1-2 are executed simultaneously.tea.Sequence
, so Command3-2-1 and Command3-2-2 are executed in sequence.This is a simple sequence diagram for Command1.
In the figure below, the solid black line represents the go routine, the solid red line represents the invocation of the go routine, and the dashed red line represents the end of the go routine being notified to the go routine that invoked the go routine just finished.
This is a simple sequence diagram for Command3.
In the current implementation, the above commands are executed as shown below.
In this section of tea.go, the bubble tea works like sequence diagram above.
The reason processing is not executed in the specified order is that
eventloop
is asked to execute allcmd()
's results asynchronously usingp.Send()
without checking whether they aresequenceMsg
orBatchMsg
or other Messages.The problem and its cause when Cmd3 is executed with the current implementation is discussed in #680 and #843. Briefly, when
sequenceMsg
is nested insequenceMsg
, the innersequenceMsg
is sent toeventloop
withp.Send
, so it cannot maintain proper order with the outersequenceMsg
.Describe the solution you'd like
improve implementation of #843. To execute the command as specified for a
sequenceMsg
orBatchMsg
of any depth of nesting, the command should be executed using a recursive function.Describe alternatives you've considered
Since the current implementation supports nesting by directly examining the result of the command invocation and nesting if statements instead of using a recursive function, the if statements must also be nested by the corresponding depth of nesting.
For now, I have not come up with an alternative.
Additional context
breaking change
Consideration should be given so that this change is not breaking. This modification changes the order in which the Command is invoked. However, in the current implementation, Command was simply executed asynchronously with
p.Send
without adhering to the order specified bytea.Sequence
ortea.Batch
. Therefore, the commands in the program that work correctly at this time are not guaranteed to be in sequence. In other words, the program works even if the order is not guaranteed. Therefore, even if this modification guarantees order, it is unlikely that a program that was already working will stop working. On the other hand, the programs that will not work correctly due to this modification are programs with bugs that happen to be working well due to the unguaranteed execution order byp.Send
andeventloop
.other information
Sequence
commands #680, fix: process nested sequence messages in order #843 are related issues and pull requests. They helped me a lot.The text was updated successfully, but these errors were encountered: