fix(navigation): preserve initial transition

lib/mob/router.ex

@@ -158,7 +158,7 @@ defmodule Mob.Router do
 
  case start_screen(screen_module, params, state) do
  {:ok, entry, state} ->
- state = make_current(state, entry)
+ state = make_current(state, entry, :none)
 
  if render_mode == :render do
  # A notification that launched the app from a killed state. Sent to
@@ -361,8 +361,8 @@ defmodule Mob.Router do
 
  # The single place `current` changes. The sender is told here and nowhere
  # else, so only the screen the user is looking at can commit a frame.
- defp make_current(state, entry) do
- Mob.Sender.set_active(entry.ref)
+ defp make_current(state, entry, transition) do
+ Mob.Sender.activate(entry.ref, transition)
  %{state | current: entry}
  end
 
@@ -465,8 +465,8 @@ defmodule Mob.Router do
  Mob.Nav.history(state.nav) != [] ->
  state = drop_entry(state, dead_pid)
  [previous | rest] = Mob.Nav.history(state.nav)
- state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous)
- paint(previous, :pop, state)
+ state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous, :pop)
+ paint(previous, :none, state)
  state
 
  true ->
@@ -485,8 +485,8 @@ defmodule Mob.Router do
  # switch/3 parks the dead entry under the outgoing stack; drop it straight
  # after, so nothing can restore a corpse by switching back.
  nav = Mob.Nav.drop_parked(nav, &(&1.pid == entry.pid))
- state = make_current(%{state | nav: nav}, live)
- paint(live, :pop, state)
+ state = make_current(%{state | nav: nav}, live, :pop)
+ paint(live, :none, state)
  state
  else
  _ ->
@@ -595,8 +595,11 @@ defmodule Mob.Router do
  # goes with it. The ones still in `rest` stay resident — that is what
  # makes pop restore prior state without re-mounting.
  state = stop_screen(state.current, state)
- state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous)
- do_paint(previous, :pop, state, mode)
+
+ state =
+ make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous, :pop)
+
+ do_paint(previous, :none, state, mode)
  state
 
  [] ->
@@ -612,8 +615,8 @@ defmodule Mob.Router do
  ]
 
  state = Enum.reduce(discarded, state, &stop_screen/2)
- state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, root)
- do_paint(root, :pop, state, mode)
+ state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, root, :pop)
+ do_paint(root, :none, state, mode)
  state
 
  [] ->
@@ -643,7 +646,7 @@ defmodule Mob.Router do
  defp apply_nav_action({:switch_tab, tab}, state, mode) do
  case Mob.Nav.switch(state.nav, tab, state.current) do
  {:switched, nav, entry} ->
- state = make_current(%{state | nav: nav}, entry)
+ state = make_current(%{state | nav: nav}, entry, :none)
  do_paint(entry, :none, state, mode)
  state
 
@@ -652,7 +655,7 @@ defmodule Mob.Router do
  # leaves navigation pointing at a stack whose screen never started.
  case start_screen(root_module, %{}, state) do
  {:ok, entry, state} ->
- state = make_current(%{state | nav: nav}, entry)
+ state = make_current(%{state | nav: nav}, entry, :none)
  do_paint(entry, :none, state, mode)
  state
 
@@ -684,8 +687,8 @@ defmodule Mob.Router do
  case start_screen(new_module, mount_params, state) do
  {:ok, entry, state} ->
  nav = Mob.Nav.put_history(state.nav, [state.current | Mob.Nav.history(state.nav)])
- state = make_current(%{state | nav: nav}, entry)
- do_paint(entry, :push, state, mode)
+ state = make_current(%{state | nav: nav}, entry, :push)
+ do_paint(entry, :none, state, mode)
  state
 
  {:error, _reason} ->
@@ -698,8 +701,11 @@ defmodule Mob.Router do
  {:ok, entry, state} ->
  discarded = [state.current | Mob.Nav.history(state.nav)]
  state = Enum.reduce(discarded, state, &stop_screen/2)
- state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry)
- do_paint(entry, transition, state, mode)
+
+ state =
+ make_current(%{state | nav: Mob.Nav.put_history(state.nav, [])}, entry, transition)
+
+ do_paint(entry, :none, state, mode)
  state
 
  {:error, _reason} ->
@@ -714,8 +720,11 @@ defmodule Mob.Router do
  {:found, previous, rest} ->
  discarded = [state.current | Enum.take_while(history, &(&1.pid != previous.pid))]
  state = Enum.reduce(discarded, state, &stop_screen/2)
- state = make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous)
- do_paint(previous, :pop, state, mode)
+
+ state =
+ make_current(%{state | nav: Mob.Nav.put_history(state.nav, rest)}, previous, :pop)
+
+ do_paint(previous, :none, state, mode)
  state
 
  :not_found ->

lib/mob/sender.ex

@@ -50,6 +50,11 @@ defmodule Mob.Sender do
  `sync/1` that merely replied would return before the frame was committed.
  Mailbox order is the wrong tool here, and it looks like the right one.
 
+ `Mob.Router` uses `activate/2` before asking a screen to paint. Activation is
+ synchronous and carries the navigation transition as a one-shot reservation,
+ so an ordinary repaint from the newly active screen cannot race ahead and
+ erase the animation. Whichever tree arrives first consumes the reservation.
+
  `Mob.Router` uses `sync/1` on its `handle_call` paths to keep the guarantee
  `Mob.Test` documents for the synchronous navigation helpers. Note the ordering
  guarantee only covers renders cast by the *calling* process; the BEAM promises
@@ -68,7 +73,7 @@ defmodule Mob.Sender do
  """
  @type screen_ref :: reference() | atom()
 
- defstruct active: nil, pending: %{}
+ defstruct active: nil, pending: %{}, reserved_transition: nil
 
  @doc "Start the sender. Named, so there is exactly one."
  @spec start_link(keyword()) :: GenServer.on_start()
@@ -114,6 +119,21 @@ defmodule Mob.Sender do
  @spec set_active(screen_ref()) :: :ok
  def set_active(ref), do: GenServer.cast(__MODULE__, {:set_active, ref})
 
+ @doc """
+ Activate a screen and reserve its navigation transition for the next frame.
+
+ Unlike `set_active/1`, this call is synchronous. The router uses it at the
+ navigation boundary so paints sent by different screen processes cannot be
+ observed before the transition intent. The first tree for `ref` consumes the
+ reservation; later ordinary repaints remain `:none`.
+
+ A `:none` transition only activates the screen and creates no reservation.
+ """
+ @spec activate(screen_ref(), atom()) :: :ok
+ def activate(ref, transition) do
+ if running?(), do: GenServer.call(__MODULE__, {:activate, ref, transition}), else: :ok
+ end
+
  @doc """
  Queue `tree` for commit on behalf of screen `ref`.
 
@@ -146,20 +166,47 @@ defmodule Mob.Sender do
  end
 
  @impl GenServer
- def handle_cast({:set_active, ref}, state), do: {:noreply, %{state | active: ref}}
+ def handle_call({:activate, ref, transition}, _from, state) do
+ reserved_transition = if transition == :none, do: nil, else: {ref, transition}
+ {:reply, :ok, %{state | active: ref, reserved_transition: reserved_transition}}
+ end
+
+ def handle_call(:sync, _from, state) do
+ # Flush here rather than just replying. The `:flush` this render self-sent
+ # lands at the BACK of the mailbox, which is behind a `sync` the caller has
+ # already queued — replying without flushing would return before the frame
+ # was committed, which is the one thing this function promises not to do.
+ {:reply, :ok, flush(state)}
+ end
+
+ @impl GenServer
+ def handle_cast({:set_active, ref}, state) do
+ {:noreply, %{state | active: ref, reserved_transition: nil}}
+ end
 
  def handle_cast({:render, ref, tree, platform, nif, transition}, state) do
  # Overwrite rather than append: a newer tree for the same screen supersedes
  # the one waiting, which is the whole point of queueing here. The transition
  # is the exception — it describes the navigation animation for this frame,
  # not the frame's content, so a push superseded by an ordinary re-render
  # still has to animate as a push or the transition is silently swallowed.
- transition = carry_transition(state.pending, ref, transition)
+ {transition, reserved_transition} =
+ take_transition(state.pending, state.reserved_transition, ref, transition)
+
  pending = Map.put(state.pending, ref, {tree, platform, nif, transition})
  send(self(), :flush)
- {:noreply, %{state | pending: pending}}
+ {:noreply, %{state | pending: pending, reserved_transition: reserved_transition}}
  end
 
+ defp take_transition(pending, {ref, reserved}, ref, :none),
+ do: {carry_transition(pending, ref, reserved), nil}
+
+ defp take_transition(pending, {ref, _reserved}, ref, transition),
+ do: {carry_transition(pending, ref, transition), nil}
+
+ defp take_transition(pending, reserved, ref, transition),
+ do: {carry_transition(pending, ref, transition), reserved}
+
  defp carry_transition(pending, ref, :none) do
  case Map.fetch(pending, ref) do
  {:ok, {_tree, _platform, _nif, superseded}} -> superseded
@@ -174,15 +221,6 @@ defmodule Mob.Sender do
 
  def handle_info(_message, state), do: {:noreply, state}
 
- @impl GenServer
- def handle_call(:sync, _from, state) do
- # Flush here rather than just replying. The `:flush` this render self-sent
- # lands at the BACK of the mailbox, which is behind a `sync` the caller has
- # already queued — replying without flushing would return before the frame
- # was committed, which is the one thing this function promises not to do.
- {:reply, :ok, flush(state)}
- end
-
  defp flush(state) do
  case Map.fetch(state.pending, state.active) do
  {:ok, payload} -> commit(payload)

test/mob/screen_sender_wiring_test.exs

@@ -2,9 +2,9 @@ defmodule Mob.ScreenSenderWiringTest do
  @moduledoc """
  The seam between `Mob.Screen` and `Mob.Sender`.
 
- Screens run `:no_render` here, so no tree is ever committed — but
-`Mob.Sender.set_active/1` is a cast, so with a real sender running these
-assertions pin *who* declares the active screen and *when*.
+ Screens run `:no_render` here, so no tree is ever committed — but with a real
+sender running these assertions pin *who* declares the active screen and
+ *when*.
 
  Since MOB-112 the sender's key is per **screen**, not per stack. Every screen
  is a live process that repaints on any message it receives, including the ones
@@ -131,7 +131,7 @@ defmodule Mob.ScreenSenderWiringTest do
  Sender.sync()
  settings_ref = current_ref(screen)
 
- # A screen re-rendering must not promote itself — this fails if set_active/1
+ # A screen re-rendering must not promote itself — this fails if activation
  # moves back into the render path.
  Mob.Screen.dispatch(screen, "bump", %{})
  Sender.sync()

test/mob/sender_test.exs

@@ -144,6 +144,70 @@ defmodule Mob.SenderTest do
  end
 
  describe "coalescing preserves the transition" do
+ test "an immediate first paint cannot overtake its navigation transition" do
+ start_sender(:home)
+
+ assert :ok = Sender.activate(:details, :push)
+ Sender.render(:details, tree("mounted"), :android, RecordingNif, :none)
+ Sender.sync()
+
+ Sender.render(:details, tree("loaded"), :android, RecordingNif, :none)
+ Sender.sync()
+
+ transitions = for {:set_transition, transition} <- RecordingNif.calls(), do: transition
+
+ assert transitions == [:push, :none]
+
+ assert [mounted, loaded] = committed_texts()
+ assert mounted =~ "mounted"
+ assert loaded =~ "loaded"
+ end
+
+ test "an ordinary first paint consumes the activated navigation transition" do
+ state = %Sender{active: :home}
+
+ {:reply, :ok, state} = Sender.handle_call({:activate, :details, :push}, self(), state)
+
+ {:noreply, state} =
+ Sender.handle_cast({:render, :details, tree("first"), :ios, RecordingNif, :none}, state)
+
+ assert state.reserved_transition == nil
+ {:noreply, _state} = Sender.handle_info(:flush, state)
+
+ assert {:set_transition, :push} in RecordingNif.calls()
+ end
+
+ test "the activated transition survives a second ordinary paint before flush" do
+ state = %Sender{active: :home}
+
+ {:reply, :ok, state} = Sender.handle_call({:activate, :details, :push}, self(), state)
+
+ {:noreply, state} =
+ Sender.handle_cast({:render, :details, tree("first"), :ios, RecordingNif, :none}, state)
+
+ {:noreply, state} =
+ Sender.handle_cast({:render, :details, tree("latest"), :ios, RecordingNif, :none}, state)
+
+ {:noreply, _state} = Sender.handle_info(:flush, state)
+
+ assert {:set_transition, :push} in RecordingNif.calls()
+ assert [json] = committed_texts()
+ assert json =~ "latest"
+ end
+
+ test "activation without a transition leaves the first paint ordinary" do
+ state = %Sender{active: :home}
+
+ {:reply, :ok, state} = Sender.handle_call({:activate, :settings, :none}, self(), state)
+
+ {:noreply, state} =
+ Sender.handle_cast({:render, :settings, tree("first"), :ios, RecordingNif, :none}, state)
+
+ {:noreply, _state} = Sender.handle_info(:flush, state)
+
+ assert {:set_transition, :none} in RecordingNif.calls()
+ end
+
  test "a push superseded by an ordinary re-render still animates as a push" do
  state = %Sender{active: :home}