ソースを参照

fix chaining actions (#144)

* fix chaining actions

* improvements
Chris Tate 4 ヶ月 前
コミット
a110c6e0ea

+ 3 - 3
packages/react-native/src/contexts/actions.tsx

@@ -135,7 +135,7 @@ export function ActionProvider({
   navigate,
   children,
 }: ActionProviderProps) {
-  const { state, get, set } = useStateStore();
+  const { get, set, getSnapshot } = useStateStore();
   const [handlers, setHandlers] =
     useState<Record<string, ActionHandler>>(initialHandlers);
   const [loadingActions, setLoadingActions] = useState<Set<string>>(new Set());
@@ -151,7 +151,7 @@ export function ActionProvider({
 
   const execute = useCallback(
     async (binding: ActionBinding) => {
-      const resolved = resolveAction(binding, state);
+      const resolved = resolveAction(binding, getSnapshot());
 
       // Built-in: setState updates the StateProvider state directly
       if (resolved.action === "setState" && resolved.params) {
@@ -298,7 +298,7 @@ export function ActionProvider({
         });
       }
     },
-    [state, handlers, get, set, navigate],
+    [handlers, get, set, getSnapshot, navigate],
   );
 
   const confirm = useCallback(() => {

+ 12 - 7
packages/react-native/src/renderer.tsx

@@ -158,6 +158,7 @@ const ElementRenderer = React.memo(function ElementRenderer({
   const repeatScope = useRepeatScope();
   const { ctx } = useVisibility();
   const { execute } = useActions();
+  const { getSnapshot } = useStateStore();
 
   // Build context with repeat scope (used for both visibility and props)
   const fullCtx: PropResolutionContext = useMemo(
@@ -183,25 +184,29 @@ const ElementRenderer = React.memo(function ElementRenderer({
   // Must be called before any early return to satisfy Rules of Hooks.
   const onBindings = element.on;
   const emit = useCallback(
-    (eventName: string) => {
+    async (eventName: string) => {
       const binding = onBindings?.[eventName];
       if (!binding) return;
       const actionBindings = Array.isArray(binding) ? binding : [binding];
       for (const b of actionBindings) {
         if (!b.params) {
-          execute(b);
+          await execute(b);
           continue;
         }
-        // Resolve all action params via resolveActionParam which handles
-        // $item (→ absolute state path), $index (→ number), $state, $cond, and literals.
+        // Build a fresh context with live store state so that $state
+        // references in later actions see mutations from earlier ones.
+        const liveCtx: PropResolutionContext = {
+          ...fullCtx,
+          stateModel: getSnapshot(),
+        };
         const resolved: Record<string, unknown> = {};
         for (const [key, val] of Object.entries(b.params)) {
-          resolved[key] = resolveActionParam(val, fullCtx);
+          resolved[key] = resolveActionParam(val, liveCtx);
         }
-        execute({ ...b, params: resolved });
+        await execute({ ...b, params: resolved });
       }
     },
-    [onBindings, execute, fullCtx],
+    [onBindings, execute, fullCtx, getSnapshot],
   );
 
   // Don't render if not visible

+ 3 - 3
packages/react-pdf/src/contexts/actions.tsx

@@ -90,7 +90,7 @@ export function ActionProvider({
   navigate,
   children,
 }: ActionProviderProps) {
-  const { state, get, set } = useStateStore();
+  const { get, set, getSnapshot } = useStateStore();
   const [handlers, setHandlers] =
     useState<Record<string, ActionHandler>>(initialHandlers);
   const [loadingActions, setLoadingActions] = useState<Set<string>>(new Set());
@@ -106,7 +106,7 @@ export function ActionProvider({
 
   const execute = useCallback(
     async (binding: ActionBinding) => {
-      const resolved = resolveAction(binding, state);
+      const resolved = resolveAction(binding, getSnapshot());
 
       if (resolved.action === "setState" && resolved.params) {
         const statePath = resolved.params.statePath as string;
@@ -211,7 +211,7 @@ export function ActionProvider({
         });
       }
     },
-    [state, handlers, get, set, navigate],
+    [handlers, get, set, getSnapshot, navigate],
   );
 
   const confirm = useCallback(() => {

+ 12 - 5
packages/react-pdf/src/renderer.tsx

@@ -123,6 +123,7 @@ const ElementRenderer = React.memo(function ElementRenderer({
   const repeatScope = useRepeatScope();
   const { ctx } = useVisibility();
   const { execute } = useActions();
+  const { getSnapshot } = useStateStore();
 
   const fullCtx: PropResolutionContext = useMemo(
     () =>
@@ -144,23 +145,29 @@ const ElementRenderer = React.memo(function ElementRenderer({
 
   const onBindings = element.on;
   const emit = useCallback(
-    (eventName: string) => {
+    async (eventName: string) => {
       const binding = onBindings?.[eventName];
       if (!binding) return;
       const actionBindings = Array.isArray(binding) ? binding : [binding];
       for (const b of actionBindings) {
         if (!b.params) {
-          execute(b);
+          await execute(b);
           continue;
         }
+        // Build a fresh context with live store state so that $state
+        // references in later actions see mutations from earlier ones.
+        const liveCtx: PropResolutionContext = {
+          ...fullCtx,
+          stateModel: getSnapshot(),
+        };
         const resolved: Record<string, unknown> = {};
         for (const [key, val] of Object.entries(b.params)) {
-          resolved[key] = resolveActionParam(val, fullCtx);
+          resolved[key] = resolveActionParam(val, liveCtx);
         }
-        execute({ ...b, params: resolved });
+        await execute({ ...b, params: resolved });
       }
     },
-    [onBindings, execute, fullCtx],
+    [onBindings, execute, fullCtx, getSnapshot],
   );
 
   if (!isVisible) {

+ 6 - 2
packages/react-state/src/index.tsx

@@ -30,6 +30,8 @@ export interface StateContextValue {
   set: (path: string, value: unknown) => void;
   /** Update multiple values at once */
   update: (updates: Record<string, unknown>) => void;
+  /** Return the live state snapshot from the underlying store (not the React render snapshot). */
+  getSnapshot: () => StateModel;
 }
 
 const StateContext = createContext<StateContextValue | null>(null);
@@ -170,9 +172,11 @@ export function StateProvider({
 
   const get = useCallback((path: string) => storeRef.current.get(path), []);
 
+  const getSnapshot = useCallback(() => storeRef.current.getSnapshot(), []);
+
   const value = useMemo<StateContextValue>(
-    () => ({ state, get, set, update }),
-    [state, get, set, update],
+    () => ({ state, get, set, update, getSnapshot }),
+    [state, get, set, update, getSnapshot],
   );
 
   return (

+ 198 - 0
packages/react/src/chained-actions.test.tsx

@@ -0,0 +1,198 @@
+import { describe, it, expect } from "vitest";
+import React from "react";
+import { render, act, fireEvent, screen } from "@testing-library/react";
+import type { Spec } from "@json-render/core";
+import {
+  JSONUIProvider,
+  Renderer,
+  type ComponentRenderProps,
+} from "./renderer";
+import { useStateStore } from "./contexts/state";
+
+/**
+ * Minimal Button component that calls emit("press") on click.
+ */
+function Button({ element, emit }: ComponentRenderProps<{ label: string }>) {
+  return (
+    <button data-testid="btn" onClick={() => emit("press")}>
+      {element.props.label}
+    </button>
+  );
+}
+
+/**
+ * Text component that renders its `text` prop.
+ */
+function Text({ element }: ComponentRenderProps<{ text: unknown }>) {
+  const value = element.props.text;
+  return (
+    <span data-testid={`text-${element.type}`}>
+      {typeof value === "string" ? value : JSON.stringify(value)}
+    </span>
+  );
+}
+
+/**
+ * Helper component that reads live state and exposes it via a data attribute
+ * so we can assert against the store after actions fire.
+ */
+function StateProbe() {
+  const { state } = useStateStore();
+  return <pre data-testid="state-probe">{JSON.stringify(state)}</pre>;
+}
+
+const registry = {
+  Button,
+  Text,
+};
+
+describe("chained actions: live $state resolution (#141)", () => {
+  it("setState after pushState sees the post-push value via $state", async () => {
+    const spec: Spec = {
+      state: { items: ["initial"], observed: "not yet set" },
+      root: "main",
+      elements: {
+        main: {
+          type: "Button",
+          props: { label: "Add Item" },
+          on: {
+            press: [
+              {
+                action: "pushState",
+                params: { statePath: "/items", value: "new-item" },
+              },
+              {
+                action: "setState",
+                params: {
+                  statePath: "/observed",
+                  value: { $state: "/items" },
+                },
+              },
+            ],
+          },
+        },
+      },
+    };
+
+    function App() {
+      return (
+        <JSONUIProvider registry={registry} initialState={spec.state}>
+          <Renderer spec={spec} registry={registry} />
+          <StateProbe />
+        </JSONUIProvider>
+      );
+    }
+
+    render(<App />);
+
+    await act(async () => {
+      fireEvent.click(screen.getByTestId("btn"));
+    });
+
+    const probe = screen.getByTestId("state-probe");
+    const state = JSON.parse(probe.textContent!);
+
+    expect(state.items).toEqual(["initial", "new-item"]);
+    expect(state.observed).toEqual(["initial", "new-item"]);
+  });
+
+  it("multiple pushState + setState chain resolves correctly", async () => {
+    const spec: Spec = {
+      state: { items: [], snapshot: null },
+      root: "main",
+      elements: {
+        main: {
+          type: "Button",
+          props: { label: "Go" },
+          on: {
+            press: [
+              {
+                action: "pushState",
+                params: { statePath: "/items", value: "a" },
+              },
+              {
+                action: "pushState",
+                params: { statePath: "/items", value: "b" },
+              },
+              {
+                action: "setState",
+                params: {
+                  statePath: "/snapshot",
+                  value: { $state: "/items" },
+                },
+              },
+            ],
+          },
+        },
+      },
+    };
+
+    function App() {
+      return (
+        <JSONUIProvider registry={registry} initialState={spec.state}>
+          <Renderer spec={spec} registry={registry} />
+          <StateProbe />
+        </JSONUIProvider>
+      );
+    }
+
+    render(<App />);
+
+    await act(async () => {
+      fireEvent.click(screen.getByTestId("btn"));
+    });
+
+    const state = JSON.parse(screen.getByTestId("state-probe").textContent!);
+
+    expect(state.items).toEqual(["a", "b"]);
+    expect(state.snapshot).toEqual(["a", "b"]);
+  });
+
+  it("setState reading a path mutated by an earlier setState sees fresh value", async () => {
+    const spec: Spec = {
+      state: { counter: 0, counterCopy: -1 },
+      root: "main",
+      elements: {
+        main: {
+          type: "Button",
+          props: { label: "Go" },
+          on: {
+            press: [
+              {
+                action: "setState",
+                params: { statePath: "/counter", value: 42 },
+              },
+              {
+                action: "setState",
+                params: {
+                  statePath: "/counterCopy",
+                  value: { $state: "/counter" },
+                },
+              },
+            ],
+          },
+        },
+      },
+    };
+
+    function App() {
+      return (
+        <JSONUIProvider registry={registry} initialState={spec.state}>
+          <Renderer spec={spec} registry={registry} />
+          <StateProbe />
+        </JSONUIProvider>
+      );
+    }
+
+    render(<App />);
+
+    await act(async () => {
+      fireEvent.click(screen.getByTestId("btn"));
+    });
+
+    const state = JSON.parse(screen.getByTestId("state-probe").textContent!);
+
+    expect(state.counter).toBe(42);
+    expect(state.counterCopy).toBe(42);
+  });
+});

+ 3 - 8
packages/react/src/contexts/actions.tsx

@@ -6,7 +6,6 @@ import React, {
   useState,
   useCallback,
   useMemo,
-  useRef,
   type ReactNode,
 } from "react";
 import {
@@ -137,11 +136,7 @@ export function ActionProvider({
   navigate,
   children,
 }: ActionProviderProps) {
-  const { state, get, set } = useStateStore();
-  // Keep a ref to the latest state so `execute` doesn't change on every
-  // state update — preventing the entire action context from churning.
-  const stateRef = useRef(state);
-  stateRef.current = state;
+  const { get, set, getSnapshot } = useStateStore();
 
   const [handlers, setHandlers] =
     useState<Record<string, ActionHandler>>(initialHandlers);
@@ -158,7 +153,7 @@ export function ActionProvider({
 
   const execute = useCallback(
     async (binding: ActionBinding) => {
-      const resolved = resolveAction(binding, stateRef.current);
+      const resolved = resolveAction(binding, getSnapshot());
 
       // Built-in: setState updates the StateProvider state directly
       if (resolved.action === "setState" && resolved.params) {
@@ -305,7 +300,7 @@ export function ActionProvider({
         });
       }
     },
-    [handlers, get, set, navigate],
+    [handlers, get, set, getSnapshot, navigate],
   );
 
   const confirm = useCallback(() => {

+ 12 - 7
packages/react/src/renderer.tsx

@@ -158,6 +158,7 @@ const ElementRenderer = React.memo(function ElementRenderer({
   const repeatScope = useRepeatScope();
   const { ctx } = useVisibility();
   const { execute } = useActions();
+  const { getSnapshot } = useStateStore();
 
   // Build context with repeat scope (used for both visibility and props)
   const fullCtx: PropResolutionContext = useMemo(
@@ -183,25 +184,29 @@ const ElementRenderer = React.memo(function ElementRenderer({
   // Must be called before any early return to satisfy Rules of Hooks.
   const onBindings = element.on;
   const emit = useCallback(
-    (eventName: string) => {
+    async (eventName: string) => {
       const binding = onBindings?.[eventName];
       if (!binding) return;
       const actionBindings = Array.isArray(binding) ? binding : [binding];
       for (const b of actionBindings) {
         if (!b.params) {
-          execute(b);
+          await execute(b);
           continue;
         }
-        // Resolve all action params via resolveActionParam which handles
-        // $item (→ absolute state path), $index (→ number), $state, $cond, and literals.
+        // Build a fresh context with live store state so that $state
+        // references in later actions see mutations from earlier ones.
+        const liveCtx: PropResolutionContext = {
+          ...fullCtx,
+          stateModel: getSnapshot(),
+        };
         const resolved: Record<string, unknown> = {};
         for (const [key, val] of Object.entries(b.params)) {
-          resolved[key] = resolveActionParam(val, fullCtx);
+          resolved[key] = resolveActionParam(val, liveCtx);
         }
-        execute({ ...b, params: resolved });
+        await execute({ ...b, params: resolved });
       }
     },
-    [onBindings, execute, fullCtx],
+    [onBindings, execute, fullCtx, getSnapshot],
   );
 
   // Create on() function that returns an EventHandle with metadata for a specific event.