zero-swift: unregistered mutators fail instead of crashing; unnamed-query warning; docs and CLI help

.changeset/parked-decisions.md

@@ -0,0 +1,5 @@
+---
+'zero-swift': patch
+---
+
+Calling a mutator that isn't registered with the `Zero` no longer crashes the app: `mutate` logs an error and returns a result whose `client` and `server` are both a `.zeroError` with the message JS throws, and nothing is written or sent. Waiting for `.complete` or `.cached` on an unnamed query, or preloading one, now logs a warning once per query: an unnamed query only sees local data and is never complete, so use a named query to sync it. The docs of `run` and `preload` say what happens to a pending call when the client closes or is replaced. `zero-swift codegen --help` no longer prints the examples `--schemaFile shared/schema.ts` and `--check true`; it shows `--schema-file` and a bare `--check`.

packages/zero-swift-cli/src/cli.ts

@@ -151,7 +151,7 @@ export const cli = Cli.create('zero-swift', {
     "Generates what the Zero Swift client needs from a Zero app's TypeScript schema, queries, and mutators.",
 }).command('codegen', {
   description:
-    "Generate the Swift row types, schema, and query builder from the app's TypeScript schema. Options override zero-swift.json, which is read when present.",
+    "Generate the Swift row types, schema, and query builder from the app's TypeScript schema. Options override zero-swift.json, which is read when present. See swift/Sources/Zero/Zero.docc/GettingStarted.md in github.com/wevm/zero.",
   options: z.object({
     config: z
       .string()
@@ -187,20 +187,10 @@ export const cli = Cli.create('zero-swift', {
       .describe('Access level of the generated code (default public)'),
     check,
   }),
-  examples: [
-    {description: 'Generate from zero-swift.json'},
-    {
-      options: {
-        schemaFile: 'shared/schema.ts',
-        out: 'Sources/App/Schema.swift',
-      },
-      description: 'Generate without a config file',
-    },
-    {
-      options: {check: true},
-      description: 'Fail in CI when the output is stale',
-    },
-  ],
+  // incur 0.7.1 prints example options as `--camelKey value`, so the hint
+  // shows `--schema-file` and a bare `--check` instead.
+  examples: [{description: 'Generate from zero-swift.json'}],
+  hint: 'Without a config file: zero-swift codegen --schema-file shared/schema.ts --out Sources/App/Schema.swift. In CI, zero-swift codegen --check fails when the output is stale.',
   run: c =>
     reporting(c.error, async () => {
       const cwd = process.cwd();

swift/Sources/Zero/Client/Definitions.swift

@@ -268,10 +268,12 @@ public enum MutationOutcome: Sendable, Hashable {
   /// The mutator or the API server threw an `ApplicationError`.
   case appError(message: String, details: JSONValue?)
   /// Zero couldn't run the mutation or learn its result, such as while
-  /// offline or when the connection closed. On the server side this doesn't
-  /// mean the mutation wasn't applied: it may have been pushed before the
-  /// connection closed, and the client sends pending mutations again when
-  /// it reconnects. Don't retry it as if it had failed.
+  /// offline, when the connection closed, or when the mutator isn't
+  /// registered with the `Zero` (then nothing ran or was sent). Otherwise,
+  /// on the server side this doesn't mean the mutation wasn't applied: it
+  /// may have been pushed before the connection closed, and the client
+  /// sends pending mutations again when it reconnects. Don't retry it as if
+  /// it had failed.
   case zeroError(message: String)
 
   /// The JS result object.

swift/Sources/Zero/Client/Engine.swift

@@ -223,7 +223,7 @@ final class Engine {
         self?.queryManager.updateCustom(name: id.name, args: id.args, ttl: ttl)
       },
       batchViewUpdates: { $0() },
-      microtask: { loop.microtask($0) })
+      microtask: { loop.microtask($0) }, log: log)
     _ = connectionManager.subscribe { [weak self] state in self?.onConnectionStateChange(state) }
     onConnectionStateChange(connectionManager.state)
     queryManager = QueryManager(
@@ -1149,6 +1149,17 @@ final class Engine {
   func mutate(name: String, args: JSONValue?) -> MutatorResult {
     let client = Deferred<MutationOutcome>()
     let server = Deferred<MutationOutcome>()
+    // JS throws this before it looks at the connection; here both outcomes
+    // carry it, and nothing is written or pushed.
+    guard let mutator = mutators[name] else {
+      let message =
+        "Mutator \"\(name)\" is not registered. "
+        + "Mutators must be registered with the Zero constructor before use."
+      log.error(message)
+      client.resolve(.zeroError(message: message))
+      server.resolve(.zeroError(message: message))
+      return MutatorResult(client: client, server: server)
+    }
     if let rejection = mutationRejection {
       log.warn(
         "Mutation \"\(name)\" rejected because Zero is \(rejection.stateDescription). "
@@ -1157,11 +1168,6 @@ final class Engine {
       server.resolve(.zeroError(message: rejection.message))
       return MutatorResult(client: client, server: server)
     }
-    guard let mutator = mutators[name] else {
-      preconditionFailure(
-        "Mutator \"\(name)\" is not registered. Mutators must be registered with the Zero constructor before use."
-      )
-    }
     let (ephemeralID, tracked) = mutationTracker.trackMutation()
     let log = self.log
     tracked.onSettle { result in

swift/Sources/Zero/Client/QueryContext.swift

@@ -29,20 +29,39 @@ final class QueryContext {
   /// later, together (`markPipelinesReady`).
   private(set) var pipelinesReady = false
   private var pendingPipelines: [(id: Int, attach: AttachPipeline)] = []
+  private let log: Log?
+  /// The hashes of the unnamed queries already warned about.
+  private var warnedLocalOnly = Set<String>()
 
   init(
     branch: IVMBranch,
     addCustomQuery: @escaping (AST, CustomQueryID, TTL, @escaping GotCallback) -> () -> Void,
     updateCustomQuery: @escaping (CustomQueryID, TTL) -> Void,
     batchViewUpdates: @escaping (() -> Void) -> Void,
-    microtask: @escaping (@escaping () -> Void) -> Void
+    microtask: @escaping (@escaping () -> Void) -> Void,
+    log: Log? = nil
   ) {
     self.branch = branch
     self.delegate = BranchDelegate(branch)
     self.addCustomQuery = addCustomQuery
     self.updateCustomQuery = updateCustomQuery
     self.batchViewUpdatesImpl = batchViewUpdates
     self.microtask = microtask
+    self.log = log
+  }
+
+  /// Warns, once per query, that an unnamed query is waited for or
+  /// preloaded: it only sees local data, so the server never confirms it.
+  /// As in JS, where an unnamed query isn't sent to the server either
+  /// (without `enableLegacyQueries`), it isn't an error.
+  private func warnLocalOnly(_ query: AnyQuery, _ consequence: String) {
+    guard query.customQueryID == nil, let log else { return }
+    let hash = QueryHash.hashOfAST(query.ast)
+    guard warnedLocalOnly.insert(hash).inserted else { return }
+    log.warn(
+      "The unnamed query of \"\(query.ast.table)\" (hash \(hash)) only sees local data and is "
+        + "never complete: \(consequence). To sync it from the server, use a named query "
+        + "(defineQuery, passed as a QueryRequest).")
   }
 
   /// A context for the queries a mutator runs: nothing is registered with
@@ -277,6 +296,8 @@ final class QueryContext {
       result.resolve(data)
       return result
     }
+    warnLocalOnly(
+      query, "a run waiting for .\(type.rawValue) never returns unless its task is cancelled")
     // Settling releases the view, also when `Zero.run` rejects it because
     // its task was cancelled.
     result.onSettle { _ in v.destroy() }
@@ -303,6 +324,7 @@ final class QueryContext {
     let cached = Deferred<Void>()
     let complete = Deferred<Void>()
     guard let id = query.customQueryID else {
+      warnLocalOnly(query, "preloading it does nothing, and its cached and complete never settle")
       return Preload(cleanup: {}, cached: cached, complete: complete)
     }
     let cleanup = addCustomQuery(query.ast, id, ttl) { got, error in

swift/Sources/Zero/Client/ZeroClient.swift

@@ -146,6 +146,19 @@ public final class Zero<Context: Sendable> {
   /// As in JS, `.complete` and `.cached` wait through connection errors for
   /// a later connection to confirm the rows. Cancel the task to stop
   /// waiting: the run throws `CancellationError` and releases its query.
+  ///
+  /// When the client closes, with `close()` or when `Zero` replaces it
+  /// (see `ZeroOptions.onClientStateNotFound`), a run still waiting throws
+  /// a `QueryError` saying Zero was closed, as in JS. Unlike views and
+  /// preloads, it doesn't move to the new client: run it again. Don't run
+  /// queries on a closed `Zero` (in JS a closed instance can no longer
+  /// query): here a run waiting for `.complete` or `.cached` never returns.
+  ///
+  /// An unnamed query (a `Query` rather than a `QueryRequest`) only sees
+  /// local data, as in JS without `enableLegacyQueries`: the server never
+  /// confirms it, so waiting for `.complete` or `.cached` never returns
+  /// unless the task is cancelled. The client logs a warning the first time
+  /// for each such query. Use a named query (`defineQuery`) to sync it.
   public func run<R: ZeroRow, S: QueryResult>(
     _ request: QueryRequest<R, S, Context>, type: RunType = .unknown, ttl: TTL? = nil
   ) async throws -> S {
@@ -180,6 +193,17 @@ public final class Zero<Context: Sendable> {
   }
 
   /// Keeps the query's rows synced without a view, until `cleanup`.
+  ///
+  /// When `Zero` replaces its client, the preload moves to the new one, but
+  /// its `cached` and `complete` are the old client's: if still pending,
+  /// they throw a `QueryError` saying Zero was closed, as they do on
+  /// `close()`. On a closed `Zero` they never settle.
+  ///
+  /// Preloading an unnamed query (a `Query` rather than a `QueryRequest`)
+  /// does nothing, as in JS without `enableLegacyQueries`: it only sees
+  /// local data, and `cached` and `complete` never settle. The client logs a
+  /// warning the first time for each such query. Use a named query
+  /// (`defineQuery`) to sync it.
   public func preload<R: ZeroRow, S: QueryResult>(
     _ request: QueryRequest<R, S, Context>, ttl: TTL = .defaultPreloadTTL
   ) -> Preload {
@@ -196,6 +220,10 @@ public final class Zero<Context: Sendable> {
   }
 
   /// Runs a mutator on the client now and sends it to the server.
+  ///
+  /// A mutator not registered with this `Zero` doesn't run: both outcomes
+  /// are a `.zeroError` saying so, and it is logged as an error. JS throws
+  /// that error instead.
   @discardableResult
   public func mutate(_ request: MutateRequest<Context>) -> MutatorResult {
     engine.loop.call { engine.mutate(name: request.name, args: request.args) }

swift/Sources/Zero/Zero.docc/JSParity.md

@@ -56,6 +56,13 @@ When the server no longer has the client's state, JS asks the app to
 reload. Swift replaces the client inside the same `Zero` instead: views,
 live queries, and preloads move to the new client and keep their identity.
 
+A pending `run`, and a preload's pending `cached` and `complete`, belong to
+the client that started them. When that client closes, by `close()` or by
+being replaced, they throw a ``QueryError`` saying Zero was closed, as they
+reject in JS. After a replacement, run it again: the same `Zero` now uses
+the new client. As in JS, don't query a closed `Zero`: a `run` that waits
+for `.complete` or `.cached` there never returns.
+
 ## Queries
 
 | JS                                                                   | Swift                                                                                          |
@@ -78,6 +85,14 @@ Comparing with nil differs by form. The two-argument
 three-argument form compares as JS compares `undefined`, so
 `where(\.ownerID, .eq, nil)` matches no row.
 
+An unnamed query, a `Query` such as `zql.issue` rather than a
+``QueryRequest`` from `defineQuery`, only sees local data, as in JS without
+`enableLegacyQueries`. It is for `tx.run` in mutators and for local reads.
+The server never confirms it, so `run(query, type: .complete)` (or
+`.cached`) never returns unless its task is cancelled, and `preload` of it
+does nothing. The client logs a warning the first time it sees each such
+query.
+
 ## Mutators
 
 | JS                                                                                 | Swift                                                                                                |
@@ -88,6 +103,11 @@ three-argument form compares as JS compares `undefined`, so
 | `tx.location`, `tx.reason`, `tx.clientID`, `tx.mutationID`                         | the same                                                                                             |
 | `throw new ApplicationError(message, {details})`                                   | `throw ApplicationError(message, details:)`                                                          |
 
+Calling a mutator that isn't registered with the `Zero` throws in JS. In
+Swift `mutate` returns a ``MutatorResult`` whose `client` and `server` are
+both `.zeroError` with the same message, and logs it as an error. Nothing
+is written or sent.
+
 ## React and SolidJS
 
 `ZeroUI` covers the same ground for SwiftUI, UIKit, and AppKit.

swift/Tests/ZeroTests/ClientAPITests.swift

@@ -122,6 +122,31 @@ import ZeroTesting
     #expect(try await zero.run(zql.issue).map(\.title) == ["t"])
   }
 
+  @Test func anUnregisteredMutatorFailsWithoutWritingOrPushing() async throws {
+    let zero = try makeZero()
+    scheduler.runDueTimers()
+    let socket = try #require(sockets.sockets.first)
+    socket.receive(text: #"["connected",{"wsid":"w1"}]"#)
+    scheduler.runDueTimers()
+    let unregistered = defineMutator("issue.unregistered") { (tx, args: CreateIssue, _: Auth) in
+      try tx.mutate.issue.insert([
+        "id": .string(args.id), "title": .string(args.title), "description": "",
+        "closed": false, "ownerId": .null, "createdAt": 1,
+      ])
+    }
+
+    let result = zero.mutate(unregistered(CreateIssue(id: "i4", title: "t")))
+    scheduler.runDueTimers()
+
+    let error = MutationOutcome.zeroError(
+      message: "Mutator \"issue.unregistered\" is not registered. "
+        + "Mutators must be registered with the Zero constructor before use.")
+    #expect(try await result.client.value == error)
+    #expect(try await result.server.value == error)
+    #expect(try await zero.run(zql.issue).isEmpty)
+    #expect(sent("push").isEmpty)
+  }
+
   @Test func dropsTheQueryOfAViewThatIsNoLongerReferenced() throws {
     var zero: Zero<Auth>? = try makeZero()
     var dropped: MaterializedView? = try zero!.materialize(Self.mine())