From dd6f9a94daf591bc81ebbeb11acef26b0f320fa7 Mon Sep 17 00:00:00 2001 From: Norman Fomferra Date: Sat, 20 Jun 2026 12:06:53 +0200 Subject: [PATCH] No longer need get/set inbuilt query/action --- remotestate-py/CHANGES.md | 8 ++- remotestate-py/README.md | 9 ++- remotestate-py/src/remotestate/context.py | 5 ++ remotestate-py/src/remotestate/protocol.py | 40 +++++++++++-- remotestate-py/src/remotestate/server.py | 28 +++++++--- remotestate-py/src/remotestate/service.py | 60 +++++--------------- remotestate-py/tests/test_protocol.py | 14 +++++ remotestate-py/tests/test_server.py | 65 +++++++++++++--------- remotestate-py/tests/test_service.py | 65 +++++++++++++--------- remotestate-ts/CHANGES.md | 2 + remotestate-ts/src/lib/local.ts | 13 +---- remotestate-ts/src/lib/protocol.ts | 52 ++++++++++++++++- remotestate-ts/src/lib/service.ts | 3 +- remotestate-ts/src/lib/store.ts | 25 +++++---- remotestate-ts/src/lib/types.ts | 6 +- remotestate-ts/src/test/local.test.ts | 24 -------- remotestate-ts/src/test/service.test.ts | 34 +++++++++++ remotestate-ts/src/test/store.test.ts | 22 ++++---- 18 files changed, 294 insertions(+), 181 deletions(-) diff --git a/remotestate-py/CHANGES.md b/remotestate-py/CHANGES.md index 3a41de4..7c7c31d 100644 --- a/remotestate-py/CHANGES.md +++ b/remotestate-py/CHANGES.md @@ -2,8 +2,12 @@ - `Store` now accepts any root state value, exposes it through the typed `state` property, and supports root reads/writes with the empty path. -- `Store.get()` and the built-in `Service.get()` query now default to the root - state value when no path is passed. +- `Store.get()` now defaults to the root state value when no path is passed. +- Removed the built-in `Service.get()` query and `Service.set()` action. + Store reads and writes now use dedicated `get`/`set` protocol messages, while + service actions and queries are reserved for domain methods. +- `Service` is now generic over the root state type, so `service.store` + preserves the `Store[T]` type. - Added notebook-friendly `Store.__getitem__()` and `Store.__setitem__()` aliases: - `store["items[0].label"]` uses RemoteState string path syntax. diff --git a/remotestate-py/README.md b/remotestate-py/README.md index ed8723a..cc9106c 100644 --- a/remotestate-py/README.md +++ b/remotestate-py/README.md @@ -147,15 +147,14 @@ class Counter(rs.Service): ## Service Helpers -`Service` also provides built-in methods that power the generic TypeScript bridge: +`Service` exposes the store and progress helper used by actions and queries: -- `get(path="")` reads a store value by path -- `set(path, value)` writes a store value by path - `notify(name=None, detail=None, progress=None)` emits `update_task` progress messages for tracked calls -The reserved service method names are `get`, `set`, and `notify`. Do not reuse those names -for custom actions or queries. +The reserved service method name is `notify`. Store reads and writes use +`service.store.get(...)` and `service.store.set(...)`; `get` and `set` remain +available for custom actions or queries. `Service._init_app(app)` can be overridden to customize the FastAPI app when `serve()` creates one. diff --git a/remotestate-py/src/remotestate/context.py b/remotestate-py/src/remotestate/context.py index 8c84eff..0bde24c 100644 --- a/remotestate-py/src/remotestate/context.py +++ b/remotestate-py/src/remotestate/context.py @@ -29,3 +29,8 @@ class _CallContext: "_call_context", default=None ) """Task-local context for the currently executing action or query.""" + +_suppress_store_broadcast: ContextVar[bool] = ContextVar( + "_suppress_store_broadcast", default=False +) +"""Whether store subscribers should skip external transport broadcasts.""" diff --git a/remotestate-py/src/remotestate/protocol.py b/remotestate-py/src/remotestate/protocol.py index 837cbe7..9031d56 100644 --- a/remotestate-py/src/remotestate/protocol.py +++ b/remotestate-py/src/remotestate/protocol.py @@ -17,7 +17,23 @@ class GetMessage(BaseModel): """An internal get-ID.""" path: str - """The modification path using a simplified JSON-Path format.""" + """Path into store's state using a simplified JSON-Path format.""" + + +class SetMessage(BaseModel): + """Set one store value by path.""" + + type: Literal["set"] = "set" + """Message type.""" + + call_id: str + """An internal set-ID.""" + + path: str + """Path into store's state using a simplified JSON-Path format.""" + + value: Any + """New value to assign at ``path``.""" class ActionMessage(BaseModel): @@ -99,12 +115,25 @@ class ActionResultMessage(BaseModel): """Message type.""" call_id: str - """An internal action- or query-ID.""" + """An internal action-ID.""" updates: dict[str, Any] """Mapping from state paths to changed state values. May be empty.""" +class SetResultMessage(BaseModel): + """Return the batched store updates produced by a previous ``SetMessage``.""" + + type: Literal["set_result"] = "set_result" + """Message type.""" + + call_id: str + """An internal set-ID.""" + + updates: dict[str, Any] + """Mapping from state paths to changed state values.""" + + class QueryResultMessage(BaseModel): """Return the computed result for a previous ``QueryMessage``.""" @@ -153,13 +182,13 @@ class TaskUpdateMessage(BaseModel): class ErrorMessage(BaseModel): - """Return an error for a previous action, query, or parse failure.""" + """Return an error for a previous request or parse failure.""" type: Literal["error"] = "error" """Message type.""" call_id: str - """An internal action- or query-ID.""" + """An internal request ID.""" message: str """Error message text.""" @@ -170,13 +199,14 @@ class ErrorMessage(BaseModel): # ---------------------------------------------------- IncomingMessage = Annotated[ - GetMessage | ActionMessage | QueryMessage, + GetMessage | SetMessage | ActionMessage | QueryMessage, Field(discriminator="type"), ] """Any message that can be sent from JavaScript to Python.""" OutgoingMessage = Annotated[ GetResultMessage + | SetResultMessage | QueryResultMessage | TaskUpdateMessage | ActionResultMessage diff --git a/remotestate-py/src/remotestate/server.py b/remotestate-py/src/remotestate/server.py index c53ac70..6188e05 100644 --- a/remotestate-py/src/remotestate/server.py +++ b/remotestate-py/src/remotestate/server.py @@ -15,15 +15,17 @@ GetMessage, GetResultMessage, IncomingMessage, + SetMessage, + SetResultMessage, ActionResultMessage, OutgoingMessage, QueryMessage, QueryResultMessage, TaskUpdateMessage, ) -from .context import _call_context +from .context import _suppress_store_broadcast from .service import Service -from .store import PendingUpdates +from .store import PendingUpdates, _batch_pending_updates from .transport import Transport from .log import LOG @@ -83,13 +85,13 @@ async def sender(msg: TaskUpdateMessage) -> None: return sender def _broadcast_store_update(self, updates: PendingUpdates) -> None: - # Action dispatch already returns the batched updates as an - # ActionResultMessage. This subscription is for Python-side store.set() - # calls that happen outside a dispatched service action. - if _call_context.get() is not None: + # Dispatched actions and store set messages return their updates in the + # matching result message. This subscription is for Python-side + # store.set() calls that happen outside a dispatched request. + if _suppress_store_broadcast.get(): return self._transport.send_nowait( - ActionResultMessage(call_id="store_update", updates=updates) + SetResultMessage(call_id="store_update", updates=updates) ) # noinspection PyProtectedMember @@ -112,6 +114,18 @@ async def __dispatch(self, msg: IncomingMessage) -> None: GetResultMessage(call_id=call_id, path=path, value=value) ) + case SetMessage(call_id=call_id, path=path, value=value): + token = _suppress_store_broadcast.set(True) + try: + with _batch_pending_updates() as pending: + self._store.set(path, value) + self._store._flush(pending) + await self._transport.send( + SetResultMessage(call_id=call_id, updates=pending) + ) + finally: + _suppress_store_broadcast.reset(token) + case ActionMessage( call_id=call_id, task_id=task_id, diff --git a/remotestate-py/src/remotestate/service.py b/remotestate-py/src/remotestate/service.py index 45f68d7..2a3ef70 100644 --- a/remotestate-py/src/remotestate/service.py +++ b/remotestate-py/src/remotestate/service.py @@ -4,14 +4,16 @@ import functools import inspect from collections.abc import Callable, Coroutine -from typing import Any +from typing import Any, Generic, TypeVar from fastapi import FastAPI -from .context import _call_context, _CallContext +from .context import _call_context, _CallContext, _suppress_store_broadcast from .protocol import TaskUpdateMessage from .store import PendingUpdates, Store, _batch_pending_updates +T = TypeVar("T") + class _ActionMarker: """Marker object used while collecting ``@action`` methods.""" @@ -66,19 +68,16 @@ def query(fn: Callable) -> _QueryMarker: return _QueryMarker(_ensure_async(fn)) -_BUILTIN_SERVICE_METHODS = {"get", "set", "notify"} +_RESERVED_SERVICE_METHODS = {"notify"} -class Service: +class Service(Generic[T]): """Implements the Python queries and actions exposed over the websocket bridge. Subclasses define ``@action`` and ``@query`` methods. Dispatch helpers take care of call scoping, read-only enforcement for queries, and batched store invalidation after actions complete. - The base class also provides the built-in ``get`` query and ``set`` - action used by the generic React bridge. - ``Service`` may serve as a base class for store-specific queries and actions, but it can also be instantiated as-is, if no queries and actions are required for a given store. @@ -88,15 +87,13 @@ class Service: - ``_init_app`` - FastAPI instance initialization - ``store`` - property that provides reactive state container - - ``get`` - built-in query to get a state value - - ``set`` - built-in action to set a state value - ``notify`` - report task updates Args: store: The reactive state container. """ - _store: Store + _store: Store[T] _actions: dict[str, Callable] _queries: dict[str, Callable] @@ -110,9 +107,9 @@ def __init_subclass__(cls, **kwargs: Any) -> None: cls._queries.update(getattr(base, "_queries", {})) for name, value in list(cls.__dict__.items()): - if name in _BUILTIN_SERVICE_METHODS: + if name in _RESERVED_SERVICE_METHODS: raise TypeError( - f"{cls.__name__}.{name} conflicts with a built-in " + f"{cls.__name__}.{name} conflicts with a reserved " "RemoteState service method" ) if isinstance(value, _ActionMarker): @@ -122,7 +119,7 @@ def __init_subclass__(cls, **kwargs: Any) -> None: cls._queries[name] = value.fn setattr(cls, name, value.fn) - def __init__(self, store: Store) -> None: + def __init__(self, store: Store[T]) -> None: """Create a service bound to a reactive store. Args: @@ -149,43 +146,10 @@ def _init_app(self, app: FastAPI): """ @property - def store(self) -> Store: + def store(self) -> Store[T]: """Store: The reactive state container.""" return self._store - @query - def get(self, path: str = "") -> Any: - """Built-in query that returns a store value by path. - - This is the read side of the generic bridge used by the TypeScript - ``useRemoteState()`` hook and related helpers. - - Args: - path: RemoteState path to read. If omitted, reads the root state - value. - - Returns: - The value at ``path``, or ``None`` when the path is missing. - """ - return self.store.get(path) - - @action - def set(self, path: str, value: Any) -> None: - """Built-in action that sets a store value by path. - - This is the write-side of the generic bridge used by the TypeScript - ``useRemoteState()`` hook and related helpers, so a simple UI state does - not require a custom action on every user service. - - Args: - path: RemoteState path to write. - value: New value to assign at ``path``. - - Returns: - None. - """ - self.store.set(path, value) - # noinspection PyMethodMayBeStatic def notify( self, @@ -264,6 +228,7 @@ async def _rs_invoke_action( readonly=False, ) ) + broadcast_token = _suppress_store_broadcast.set(True) try: with _batch_pending_updates() as pending: await fn(self, *args, **kwargs) @@ -271,6 +236,7 @@ async def _rs_invoke_action( self.store._flush(pending) return pending finally: + _suppress_store_broadcast.reset(broadcast_token) _call_context.reset(token) async def _rs_invoke_query( diff --git a/remotestate-py/tests/test_protocol.py b/remotestate-py/tests/test_protocol.py index c1dda99..42c469a 100644 --- a/remotestate-py/tests/test_protocol.py +++ b/remotestate-py/tests/test_protocol.py @@ -5,6 +5,8 @@ from remotestate.protocol import ( IncomingMessage, GetMessage, + SetMessage, + SetResultMessage, OutgoingMessage, ActionMessage, ) @@ -23,6 +25,18 @@ def test_get(): ) == GetMessage(call_id="x", path="y") +def test_set(): + assert _incoming_adapter.validate_json( + to_json(type="set", call_id="x", path="count", value=7) + ) == SetMessage(call_id="x", path="count", value=7) + + +def test_set_result(): + assert _outgoing_adapter.validate_python( + {"type": "set_result", "call_id": "x", "updates": {"count": 7}} + ) == SetResultMessage(call_id="x", updates={"count": 7}) + + def test_action(): assert _incoming_adapter.validate_json( to_json( diff --git a/remotestate-py/tests/test_server.py b/remotestate-py/tests/test_server.py index 813b89b..2b6317e 100644 --- a/remotestate-py/tests/test_server.py +++ b/remotestate-py/tests/test_server.py @@ -8,6 +8,8 @@ ErrorMessage, GetMessage, GetResultMessage, + SetMessage, + SetResultMessage, ActionResultMessage, QueryMessage, QueryResultMessage, @@ -36,6 +38,11 @@ def _init_app(self, app: FastAPI): async def increment(self): self.store.set("count", self.store.get("count") + 1) + @action + async def multi_set(self): + self.store.set("count", 7) + self.store.set("user.name", "Klaus") + @query async def get_count(self) -> int: return self.store.get("count") @@ -64,7 +71,7 @@ def test_external_store_set_broadcasts_update(server): server._transport.send_nowait.assert_called_once() sent = server._transport.send_nowait.call_args[0][0] - assert isinstance(sent, ActionResultMessage) + assert isinstance(sent, SetResultMessage) assert sent.call_id == "store_update" assert sent.updates == {"count": 7} @@ -136,9 +143,10 @@ async def test_transport_close_clears_connections(): # --- Dispatch --- # The dispatcher routes incoming protocol messages to the correct handler: -# GetMessage → store.get → ValueMessage -# CallMessage → service action → (store mutation, no return value) -# InvokeMessage → service query → InvokeResultMessage +# GetMessage → store.get → GetResultMessage +# SetMessage → store.set → SetResultMessage +# ActionMessage → service action → ActionResultMessage +# QueryMessage → service query → QueryResultMessage @pytest.mark.asyncio @@ -172,44 +180,51 @@ async def test_dispatch_call_action(server): @pytest.mark.asyncio -async def test_dispatch_builtin_get_query(server): +async def test_dispatch_action_batches_store_updates(server): sent = [] server._transport.send = AsyncMock(side_effect=lambda m: sent.append(m)) + server._transport.send_nowait = MagicMock() await server._dispatch( - QueryMessage( - call_id="abc", - task_id="abc", - method="get", - args=["count"], - kwargs={}, + ActionMessage( + call_id="abc", task_id="abc", method="multi_set", args=[], kwargs={} ) ) + server._transport.send_nowait.assert_not_called() assert len(sent) == 1 - assert isinstance(sent[0], QueryResultMessage) + assert isinstance(sent[0], ActionResultMessage) assert sent[0].call_id == "abc" - assert sent[0].value == 0 + assert sent[0].updates == {"count": 7, "user.name": "Klaus"} @pytest.mark.asyncio -async def test_dispatch_builtin_set_action(server): +async def test_dispatch_set(server): sent = [] server._transport.send = AsyncMock(side_effect=lambda m: sent.append(m)) + server._transport.send_nowait = MagicMock() - await server._dispatch( - ActionMessage( - call_id="abc", - task_id="abc", - method="set", - args=["count", 7], - kwargs={}, - ) - ) + await server._dispatch(SetMessage(call_id="abc", path="count", value=7)) + assert len(sent) == 1 assert server._store.get("count") == 7 - assert isinstance(sent[0], ActionResultMessage) - assert sent[0].updates["count"] == 7 + assert isinstance(sent[0], SetResultMessage) + assert sent[0].call_id == "abc" + assert sent[0].updates == {"count": 7} + server._transport.send_nowait.assert_not_called() + + +@pytest.mark.asyncio +async def test_dispatch_set_root(server): + sent = [] + server._transport.send = AsyncMock(side_effect=lambda m: sent.append(m)) + + await server._dispatch(SetMessage(call_id="abc", path="", value={"count": 7})) + + assert len(sent) == 1 + assert server._store.get("count") == 7 + assert isinstance(sent[0], SetResultMessage) + assert sent[0].updates[""] == {"count": 7} @pytest.mark.asyncio diff --git a/remotestate-py/tests/test_service.py b/remotestate-py/tests/test_service.py index 66cef70..60b3e41 100644 --- a/remotestate-py/tests/test_service.py +++ b/remotestate-py/tests/test_service.py @@ -116,41 +116,31 @@ def invoke_query(service, method, args=None, kwargs=None): ) -# --- built-ins --- +# --- reserved names --- @pytest.mark.asyncio -async def test_builtin_get_query_works_on_base_service(store): - service = Service(store) - coro, _ = invoke_query(service, "get", args=["count"]) - result = await coro - assert result == 0 - +async def test_get_and_set_are_available_for_custom_service_methods(store): + class MyService(Service): + @query + async def get(self): + return self.store.get("count") -@pytest.mark.asyncio -async def test_builtin_get_query_defaults_to_root(store): - service = Service(store) - coro, _ = invoke_query(service, "get") - result = await coro - assert result is store.state + @action + async def set(self, count: int): + self.store.set("count", count) + service = MyService(store) + query_coro, _ = invoke_query(service, "get") + assert await query_coro == 0 -@pytest.mark.asyncio -async def test_builtin_set_action_works_on_base_service(store): - service = Service(store) - coro, _ = invoke_action(service, "set", args=["count", 7]) - await coro + action_coro, _ = invoke_action(service, "set", args=[7]) + await action_coro assert store.get("count") == 7 -def test_builtin_method_names_are_reserved(): - with pytest.raises(TypeError, match="conflicts with a built-in"): - class BadService(Service): - @query - async def get(self): - return self.store.get("count") - - with pytest.raises(TypeError, match="conflicts with a built-in"): +def test_notify_method_name_is_reserved(): + with pytest.raises(TypeError, match="conflicts with a reserved"): class BadNotifyService(Service): @action async def notify(self): @@ -186,6 +176,29 @@ async def test_action_batch_single_notify(store): assert cb.call_count == 1 +@pytest.mark.asyncio +async def test_action_does_not_notify_between_store_sets(store): + cb = MagicMock() + + class MyService(Service): + @action + async def multi_set(self): + self.store.set("count", 99) + cb.assert_not_called() + + self.store.set("user.name", "Klaus") + cb.assert_not_called() + + service = MyService(store) + store.subscribe(cb) + + coro, _ = invoke_action(service, "multi_set") + updates = await coro + + cb.assert_called_once_with({"count": 99, "user.name": "Klaus"}) + assert updates == {"count": 99, "user.name": "Klaus"} + + @pytest.mark.asyncio async def test_action_returns_updates(store): service = make_service(store) diff --git a/remotestate-ts/CHANGES.md b/remotestate-ts/CHANGES.md index 1a2f957..546e6d7 100644 --- a/remotestate-ts/CHANGES.md +++ b/remotestate-ts/CHANGES.md @@ -9,6 +9,8 @@ segment-array type. - `Store.get()` and `useRemoteStateValue()` now default to the root state value when no path is passed. +- `Store.set()` now uses a dedicated store `set` protocol message and receives + `set_result` updates instead of dispatching a service action named `set`. - Updated the transport-backed store cache so root subscriptions overlap all descendant updates, root updates materialize cached descendants, and leaf updates can patch cached root snapshots. diff --git a/remotestate-ts/src/lib/local.ts b/remotestate-ts/src/lib/local.ts index 51c1d5f..6c6f096 100644 --- a/remotestate-ts/src/lib/local.ts +++ b/remotestate-ts/src/lib/local.ts @@ -1,5 +1,4 @@ import type { RemoteStateClient } from "./client"; -import { normalizePath, PathLike } from "./path"; import { createRemoteTaskStore, type WritableTaskStore } from "./tasks"; import { type ActionMethod, @@ -90,10 +89,7 @@ export function createLocalStateClient( const ownsTaskStore = options.tasks === undefined; const actionHandlers: Partial< Record Awaitable> - > = { - ...actions, - set: createLocalSetAction(store), - }; + > = actions; const queryHandlers: Partial< Record Awaitable> > = queries; @@ -130,10 +126,3 @@ export function createLocalStateClient( }, }; } - -function createLocalSetAction(store: Store) { - const set = async (path: PathLike, value: unknown) => { - await store.set(normalizePath(path), value); - }; - return set as (...args: unknown[]) => Awaitable; -} diff --git a/remotestate-ts/src/lib/protocol.ts b/remotestate-ts/src/lib/protocol.ts index 6a514d4..b388f9d 100644 --- a/remotestate-ts/src/lib/protocol.ts +++ b/remotestate-ts/src/lib/protocol.ts @@ -22,6 +22,31 @@ export interface GetMessage { path: string; } +/** + * Write one value at a store path. + */ +export interface SetMessage { + /** + * Protocol discriminator for store set requests. + */ + type: "set"; + + /** + * Internal call ID used to correlate the response. + */ + call_id: string; + + /** + * State path to write. + */ + path: string; + + /** + * Value to assign at the path. + */ + value: unknown; +} + /** * Invoke a state-mutating service method. */ @@ -141,6 +166,26 @@ export interface ActionResultMessage { updates: Record; // path --> value mapping } +/** + * Return the batched store updates produced by a store set request. + */ +export interface SetResultMessage { + /** + * Protocol discriminator for store set results. + */ + type: "set_result"; + + /** + * Internal call ID from the request. + */ + call_id: string; + + /** + * Mapping from changed state paths to their latest values. + */ + updates: Record; +} + /** * Return the result of a previous `QueryMessage`. */ @@ -238,13 +283,18 @@ export interface ErrorMessage { /** * Any message the Remote State bridge can send to Python. */ -export type IncomingMessage = GetMessage | ActionMessage | QueryMessage; +export type IncomingMessage = + | GetMessage + | SetMessage + | ActionMessage + | QueryMessage; /** * Any message Python can send back to the Remote State bridge. */ export type OutgoingMessage = | GetResultMessage + | SetResultMessage | ActionResultMessage | QueryResultMessage | TaskUpdateMessage diff --git a/remotestate-ts/src/lib/service.ts b/remotestate-ts/src/lib/service.ts index 8f88176..d028497 100644 --- a/remotestate-ts/src/lib/service.ts +++ b/remotestate-ts/src/lib/service.ts @@ -136,10 +136,11 @@ export class ServiceImpl implements Service { if (!("call_id" in msg) || msg.call_id !== callId) { return; } - unsubscribe(); if (msg.type === "query_result") { + unsubscribe(); resolve(msg.value); } else if (msg.type === "error") { + unsubscribe(); reject(new Error(msg.message)); } }); diff --git a/remotestate-ts/src/lib/store.ts b/remotestate-ts/src/lib/store.ts index 453162b..f59039e 100644 --- a/remotestate-ts/src/lib/store.ts +++ b/remotestate-ts/src/lib/store.ts @@ -1,4 +1,8 @@ -import type { ActionResultMessage, GetResultMessage } from "./protocol"; +import type { + ActionResultMessage, + GetResultMessage, + SetResultMessage, +} from "./protocol"; import { getPathAt, formatPath, @@ -51,8 +55,8 @@ export class StoreImpl implements Store { this.unsubscribeTransport = transport.subscribe((msg) => { if (msg.type === "get_result") { this._onGetResult(msg); - } else if (msg.type === "action_result") { - this._onActionResult(msg); + } else if (msg.type === "action_result" || msg.type === "set_result") { + this._onUpdateResult(msg); } }); } @@ -69,11 +73,11 @@ export class StoreImpl implements Store { } /** - * Set a state value through the backend's built-in `set` action. + * Set a state value through the backend store protocol. * * @param path The parsed state path to write. * @param value The value to assign. - * @returns A promise that resolves after the action result is applied. + * @returns A promise that resolves after the set result is applied. */ set(path: Path, value: unknown): Promise { return new Promise((resolve, reject) => { @@ -83,7 +87,7 @@ export class StoreImpl implements Store { return; } unsubscribe(); - if (msg.type === "action_result") { + if (msg.type === "set_result") { resolve(); } else if (msg.type === "error") { reject(new Error(msg.message)); @@ -91,11 +95,10 @@ export class StoreImpl implements Store { }); this.transport.send({ - type: "action", + type: "set", call_id: callId, - method: "set", - args: [formatPath(path), value], - kwargs: {}, + path: formatPath(path), + value, }); }); } @@ -153,7 +156,7 @@ export class StoreImpl implements Store { this._notify([msg.path]); } - private _onActionResult(msg: ActionResultMessage): void { + private _onUpdateResult(msg: ActionResultMessage | SetResultMessage): void { const changedPaths = Object.keys(msg.updates); for (const [path, value] of Object.entries(msg.updates)) { this._applyUpdate(path, value); diff --git a/remotestate-ts/src/lib/types.ts b/remotestate-ts/src/lib/types.ts index 232f871..2d8a6df 100644 --- a/remotestate-ts/src/lib/types.ts +++ b/remotestate-ts/src/lib/types.ts @@ -98,9 +98,9 @@ export interface Store { /** * Set the value at a parsed state path. * - * Remote tasks dispatch the built-in backend `set` action and resolve after - * the resulting update is applied. Local tasks should update their backing - * state container and notify subscribers. + * Remote stores dispatch a store `set` message and resolve after the + * resulting update is applied. Local stores should update their backing state + * container and notify subscribers. * * @param path The parsed state path to write. An empty path replaces the * root state value. diff --git a/remotestate-ts/src/test/local.test.ts b/remotestate-ts/src/test/local.test.ts index 89a73cb..7f2ad1c 100644 --- a/remotestate-ts/src/test/local.test.ts +++ b/remotestate-ts/src/test/local.test.ts @@ -35,30 +35,6 @@ describe("createLocalStateClient", () => { await expect(client.query("count")).resolves.toBe(2); }); - it("delegates the built-in set action to the store", async () => { - const set = vi.fn(); - const client = createLocalStateClient({ - store: { - ...createStore(), - set, - }, - }); - - await client.action("set", ["count", 7]); - - expect(set).toHaveBeenCalledWith(["count"], 7); - }); - - it("rejects built-in set action without a string/array path", async () => { - const client = createLocalStateClient({ - store: createStore(), - }); - - await expect(client.action("set", [7, "count"])).rejects.toThrow( - "RemoteState path must be a string or array, but got number", - ); - }); - it("throws for unsupported local methods", async () => { const client = createLocalStateClient({ store: createStore(), diff --git a/remotestate-ts/src/test/service.test.ts b/remotestate-ts/src/test/service.test.ts index 716542d..d4761af 100644 --- a/remotestate-ts/src/test/service.test.ts +++ b/remotestate-ts/src/test/service.test.ts @@ -166,6 +166,40 @@ describe("ServiceImpl", () => { await expect(promise).resolves.toBe(15.0); }); + it("ignores progress updates before query_result", async () => { + const transport = mockTransportWithHandler(); + const taskStore = new TaskStoreImpl(); + const taskController = new TaskController( + taskStore, + asTransport(transport), + ); + const service = new ServiceImpl(asTransport(transport), taskController); + + const promise = service.query( + "compute", + [5.0], + {}, + { taskId: "compute-task" }, + ); + const sentMsg = transport.send.mock.calls[0][0] as { call_id: string }; + + transport._triggerMessage({ + type: "update_task", + call_id: sentMsg.call_id, + task_id: "compute-task", + method: "compute", + status: "running", + progress: 50, + }); + transport._triggerMessage({ + type: "query_result", + call_id: sentMsg.call_id, + value: 15.0, + }); + + await expect(promise).resolves.toBe(15.0); + }); + it("rejects on error message", async () => { const transport = mockTransportWithHandler(); const service = new ServiceImpl(asTransport(transport)); diff --git a/remotestate-ts/src/test/store.test.ts b/remotestate-ts/src/test/store.test.ts index 927c6c6..a2c6661 100644 --- a/remotestate-ts/src/test/store.test.ts +++ b/remotestate-ts/src/test/store.test.ts @@ -381,7 +381,7 @@ describe("StoreImpl", () => { ); }); - it("sets a path through the built-in set action", () => { + it("sets a path through the store set message", () => { const transport = mockTransportWithHandler(); const store = new StoreImpl(asTransport(transport)); @@ -389,15 +389,14 @@ describe("StoreImpl", () => { expect(transport.send).toHaveBeenCalledWith( expect.objectContaining({ - type: "action", - method: "set", - args: ["count", 3], - kwargs: {}, + type: "set", + path: "count", + value: 3, }), ); }); - it("sets the root path through the built-in set action", () => { + it("sets the root path through the store set message", () => { const transport = mockTransportWithHandler(); const store = new StoreImpl(asTransport(transport)); @@ -405,15 +404,14 @@ describe("StoreImpl", () => { expect(transport.send).toHaveBeenCalledWith( expect.objectContaining({ - type: "action", - method: "set", - args: ["", { count: 3 }], - kwargs: {}, + type: "set", + path: "", + value: { count: 3 }, }), ); }); - it("resolves set after matching action result", async () => { + it("resolves set after matching set result", async () => { const transport = mockTransportWithHandler(); const store = new StoreImpl(asTransport(transport)); @@ -421,7 +419,7 @@ describe("StoreImpl", () => { const sentMsg = transport.send.mock.calls[0][0] as { call_id: string }; transport._triggerMessage({ - type: "action_result", + type: "set_result", call_id: sentMsg.call_id, updates: { count: 3 }, });