fix(runtime): freeze useAnimation frame on batched pause+interval change (Ink parity) (#143)
Pausing (isActive→false) in the SAME synchronous batch as an interval change froze the frame at 0 instead of the last live frame. vue-tui split Ink's single render-time `shouldReset` into TWO `flush:"sync"` watchers; sync fires once-per-mutation, so `interval.value = X; isActive.value = false` ran the interval watcher first (while still active) → erroneous start() zeroed the frame, before the isActive watcher stop()'d. Replace them with ONE `flush:"post"` watcher on `[isActive, interval]` that coalesces the batch and fires once with the final values, mirroring Ink's `shouldReset = isActive && (intervalChanged || becameActive)` (use-animation.ts:77-96): paused → stop() (freeze, no reset); active + (becameActive || intervalChanged) → start(). `immediate:true` keeps the initial mount synchronous (one subscribe, no double-subscribe). flush:"post" was verified to fire in the BLESSED standalone (no-component) fallback: Vue's post-flush queue flushes on any reactive mutation's microtask, independent of component updates. Adds tests: batched pause+interval (both orders) freezes; resume zeros then advances at the new interval; same-interval rerender does not reset; and two standalone (no-render-tree) cases. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { PassThrough } from "node:stream";
|
import { PassThrough } from "node:stream";
|
||||||
import { defineComponent, nextTick, shallowRef, watchEffect } from "vue";
|
import { defineComponent, effectScope, nextTick, shallowRef, watchEffect } from "vue";
|
||||||
import type { ShallowRef } from "vue";
|
import type { ShallowRef } from "vue";
|
||||||
import { describe, expect, test } from "vite-plus/test";
|
import { describe, expect, test } from "vite-plus/test";
|
||||||
import { render } from "@vue-tui/testing";
|
import { render } from "@vue-tui/testing";
|
||||||
@@ -816,6 +816,97 @@ describe("useAnimation", () => {
|
|||||||
expect(f2.value).toBe(0);
|
expect(f2.value).toBe(0);
|
||||||
unmount();
|
unmount();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Ink parity (use-animation.ts:77-96): `shouldReset` is computed ONCE from
|
||||||
|
// the FINAL batched values — `isActive && (intervalChanged || becameActive
|
||||||
|
// || resetKeyChanged)`. When `isActive` ends up false, `shouldReset` is
|
||||||
|
// false regardless of an interval change in the same render, so the frame
|
||||||
|
// FREEZES at its last live value (no reset to 0). This guards the bug where
|
||||||
|
// splitting into two `flush:"sync"` watchers made the interval watcher fire
|
||||||
|
// first (while still active) and erroneously `start()` → zero the frame
|
||||||
|
// before the isActive watcher could `stop()`.
|
||||||
|
test("batched interval-change + pause in one tick freezes the frame (not 0)", async () => {
|
||||||
|
const interval = shallowRef(30);
|
||||||
|
const active = shallowRef(true);
|
||||||
|
let frame!: Readonly<ShallowRef<number>>;
|
||||||
|
const App = defineComponent(() => {
|
||||||
|
frame = useAnimation({ interval, isActive: active }).frame;
|
||||||
|
return () => <Text>{String(frame.value)}</Text>;
|
||||||
|
});
|
||||||
|
const { unmount } = await render(App);
|
||||||
|
await delay(150);
|
||||||
|
const live = frame.value;
|
||||||
|
expect(live).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
// Interval FIRST, then pause — the bug order. Both mutations land in one
|
||||||
|
// synchronous batch.
|
||||||
|
interval.value = 200;
|
||||||
|
active.value = false;
|
||||||
|
await nextTick();
|
||||||
|
|
||||||
|
// Pausing wins: the frame freezes at the last live value, NOT 0.
|
||||||
|
expect(frame.value).toBe(live);
|
||||||
|
await delay(150);
|
||||||
|
expect(frame.value).toBe(live);
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Guard: the reverse batch order (pause FIRST, then interval) must agree —
|
||||||
|
// pausing still wins and the frame freezes at the last live value.
|
||||||
|
test("batched pause + interval-change in one tick also freezes the frame", async () => {
|
||||||
|
const interval = shallowRef(30);
|
||||||
|
const active = shallowRef(true);
|
||||||
|
let frame!: Readonly<ShallowRef<number>>;
|
||||||
|
const App = defineComponent(() => {
|
||||||
|
frame = useAnimation({ interval, isActive: active }).frame;
|
||||||
|
return () => <Text>{String(frame.value)}</Text>;
|
||||||
|
});
|
||||||
|
const { unmount } = await render(App);
|
||||||
|
await delay(150);
|
||||||
|
const live = frame.value;
|
||||||
|
expect(live).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
active.value = false;
|
||||||
|
interval.value = 200;
|
||||||
|
await nextTick();
|
||||||
|
|
||||||
|
expect(frame.value).toBe(live);
|
||||||
|
await delay(150);
|
||||||
|
expect(frame.value).toBe(live);
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Guard: resuming after a batched interval-change + pause must zero and then
|
||||||
|
// advance at the NEW interval (the deferred reset lands on resume, Ink-style).
|
||||||
|
test("resume after batched interval-change + pause zeros then advances at the new interval", async () => {
|
||||||
|
const interval = shallowRef(30);
|
||||||
|
const active = shallowRef(true);
|
||||||
|
let frame!: Readonly<ShallowRef<number>>;
|
||||||
|
const App = defineComponent(() => {
|
||||||
|
frame = useAnimation({ interval, isActive: active }).frame;
|
||||||
|
return () => <Text>{String(frame.value)}</Text>;
|
||||||
|
});
|
||||||
|
const { unmount } = await render(App);
|
||||||
|
await delay(150);
|
||||||
|
expect(frame.value).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
interval.value = 200;
|
||||||
|
active.value = false;
|
||||||
|
await nextTick();
|
||||||
|
const frozen = frame.value;
|
||||||
|
expect(frozen).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
active.value = true;
|
||||||
|
await nextTick();
|
||||||
|
// Resume zeros immediately.
|
||||||
|
expect(frame.value).toBe(0);
|
||||||
|
|
||||||
|
// New interval is 200ms: after ~120ms it must still be frame 0 (one frame
|
||||||
|
// would need 200ms), proving the re-subscribe used the new interval.
|
||||||
|
await delay(120);
|
||||||
|
expect(frame.value).toBe(0);
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
// ---------------------------------------------------------------
|
// ---------------------------------------------------------------
|
||||||
@@ -1227,6 +1318,70 @@ describe("useAnimation", () => {
|
|||||||
unmount();
|
unmount();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Blessed standalone fallback (ink-divergences.md "useAnimation() outside a
|
||||||
|
// render tree drives a standalone animation"): with NO mounted component,
|
||||||
|
// useAnimation must still tick, and reactive interval/isActive changes must
|
||||||
|
// still take effect. The combined-watcher fix relies on `flush:"post"`, which
|
||||||
|
// fires standalone too — this guards that it keeps working there.
|
||||||
|
test("standalone (no render tree): ticks and reacts to live interval/isActive changes", async () => {
|
||||||
|
const scope = effectScope();
|
||||||
|
const interval = shallowRef(30);
|
||||||
|
const active = shallowRef(true);
|
||||||
|
let frame!: Readonly<ShallowRef<number>>;
|
||||||
|
scope.run(() => {
|
||||||
|
frame = useAnimation({ interval, isActive: active }).frame;
|
||||||
|
});
|
||||||
|
|
||||||
|
// Advances with no surrounding app.
|
||||||
|
await delay(150);
|
||||||
|
const live = frame.value;
|
||||||
|
expect(live).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
// Pause takes effect standalone — the frame freezes.
|
||||||
|
active.value = false;
|
||||||
|
await nextTick();
|
||||||
|
const frozen = frame.value;
|
||||||
|
await delay(120);
|
||||||
|
expect(frame.value).toBe(frozen);
|
||||||
|
|
||||||
|
// Resume + new interval take effect standalone: zero, then advance at 30ms.
|
||||||
|
interval.value = 30;
|
||||||
|
active.value = true;
|
||||||
|
await nextTick();
|
||||||
|
expect(frame.value).toBe(0);
|
||||||
|
await delay(150);
|
||||||
|
expect(frame.value).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
scope.stop();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Blessed standalone fallback, batched-pause guard: the bug's exact shape
|
||||||
|
// (interval-change + pause in one synchronous batch) must freeze, not zero,
|
||||||
|
// outside a render tree too.
|
||||||
|
test("standalone (no render tree): batched interval-change + pause freezes the frame", async () => {
|
||||||
|
const scope = effectScope();
|
||||||
|
const interval = shallowRef(30);
|
||||||
|
const active = shallowRef(true);
|
||||||
|
let frame!: Readonly<ShallowRef<number>>;
|
||||||
|
scope.run(() => {
|
||||||
|
frame = useAnimation({ interval, isActive: active }).frame;
|
||||||
|
});
|
||||||
|
|
||||||
|
await delay(150);
|
||||||
|
const live = frame.value;
|
||||||
|
expect(live).toBeGreaterThanOrEqual(1);
|
||||||
|
|
||||||
|
interval.value = 200;
|
||||||
|
active.value = false;
|
||||||
|
await nextTick();
|
||||||
|
|
||||||
|
expect(frame.value).toBe(live);
|
||||||
|
await delay(120);
|
||||||
|
expect(frame.value).toBe(live);
|
||||||
|
|
||||||
|
scope.stop();
|
||||||
|
});
|
||||||
|
|
||||||
test("renderToString renders frame 0 without throwing or leaking timers", () => {
|
test("renderToString renders frame 0 without throwing or leaking timers", () => {
|
||||||
const App = defineComponent(() => {
|
const App = defineComponent(() => {
|
||||||
const { frame } = useAnimation({ interval: 50 });
|
const { frame } = useAnimation({ interval: 50 });
|
||||||
|
|||||||
@@ -157,32 +157,45 @@ export function useAnimation(options: AnimationOptions = {}): UseAnimationReturn
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Watch isActive — when toggled to true, start (which resets values);
|
// A SINGLE watcher on BOTH (isActive, interval), mirroring Ink's render-time
|
||||||
// when toggled to false, stop (values freeze).
|
// `shouldReset = isActive && (intervalChanged || becameActive)`
|
||||||
|
// (use-animation.ts:77-96). Ink derives this once from the FINAL values of a
|
||||||
|
// render, so a batch that both changes interval AND flips isActive→false
|
||||||
|
// resolves to `shouldReset === false` and the frame FREEZES.
|
||||||
|
//
|
||||||
|
// Two `flush:"sync"` watchers cannot reproduce that: `sync` fires once PER
|
||||||
|
// mutation, so `interval.value = 200; active.value = false` in one batch ran
|
||||||
|
// the interval watcher first (while still active) → erroneous start() → frame
|
||||||
|
// zeroed, before the isActive watcher could stop(). `flush:"post"` coalesces
|
||||||
|
// the batch and fires once with the final values — verified to fire even in
|
||||||
|
// the blessed standalone fallback (no mounted component drives the post-flush
|
||||||
|
// queue, but a reactive mutation still flushes it). We always record the new
|
||||||
|
// normalized `interval` so the next start() uses it.
|
||||||
const isActive = options.isActive ?? true;
|
const isActive = options.isActive ?? true;
|
||||||
|
// immediate handles the initial mount: a single start()/stop(), no double
|
||||||
|
// subscribe (the old code split this across an immediate isActive watch + a
|
||||||
|
// non-immediate interval watch for exactly that reason).
|
||||||
watch(
|
watch(
|
||||||
() => toValue(isActive),
|
() => [toValue(isActive), normalizeInterval(toValue(options.interval))] as const,
|
||||||
(active) => {
|
([active, nextInterval], prev) => {
|
||||||
if (active) start();
|
const intervalChanged = prev !== undefined && nextInterval !== prev[1];
|
||||||
else stop();
|
const becameActive = prev === undefined || !prev[0];
|
||||||
|
interval = nextInterval;
|
||||||
|
if (!active) {
|
||||||
|
// Paused (or starting inactive): stop and freeze. Nothing to reset —
|
||||||
|
// matching reset-while-paused; the deferred zero lands on the next
|
||||||
|
// resume (which hits the `start()` below via becameActive).
|
||||||
|
stop();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Active. Ink resets+re-subscribes only when something relevant changed:
|
||||||
|
// the animation just became active, or the interval changed while active.
|
||||||
|
// (Vue's watch fires only on a real change, so on a pure re-render with an
|
||||||
|
// unchanged interval this watcher does not run at all — no reset, matching
|
||||||
|
// Ink's "rerender with same interval does not reset".)
|
||||||
|
if (becameActive || intervalChanged) start();
|
||||||
},
|
},
|
||||||
{ immediate: true, flush: "sync" },
|
{ immediate: true, flush: "post" },
|
||||||
);
|
|
||||||
|
|
||||||
// Watch the (reactive) interval. Ink's `shouldReset` is gated on isActive:
|
|
||||||
// an interval change resets + re-subscribes only when active. While inactive
|
|
||||||
// we just record the new normalized value (used by the next start()) and do
|
|
||||||
// NOT reset — there is nothing running to reset, matching reset-while-paused.
|
|
||||||
// Not `immediate`: the initial value is already applied above; immediate would
|
|
||||||
// double-subscribe with the isActive watch on mount.
|
|
||||||
watch(
|
|
||||||
() => normalizeInterval(toValue(options.interval)),
|
|
||||||
(next) => {
|
|
||||||
const wasActive = handle !== undefined;
|
|
||||||
interval = next;
|
|
||||||
if (wasActive) start();
|
|
||||||
},
|
|
||||||
{ flush: "sync" },
|
|
||||||
);
|
);
|
||||||
|
|
||||||
onScopeDispose(stop);
|
onScopeDispose(stop);
|
||||||
|
|||||||
Reference in New Issue
Block a user