Extend documentation with a guide for plan resolvers (#244)

grafast/grafast/src/steps/lambda.ts

@@ -1,3 +1,4 @@
+import { isDev } from "../dev.ts";
 import type {
   PromiseOrDirect,
   UnbatchedExecutionExtra,
@@ -8,6 +9,8 @@ import type { Step } from "../step.ts";
 import { UnbatchedStep } from "../step.ts";
 import { sideEffect } from "./sideEffect.ts";
 
+const warnedCallbacks = new WeakSet<Function>();
+
 /**
  * Calls the given lambda function for each tuple
  */
@@ -72,6 +75,15 @@ function lambda<const TInMultistep extends Multistep, TOut>(
       "lambda callback should accept one argument, perhaps you forgot to destructure the arguments?",
     );
   }
+  if (isDev && !fn.name && !warnedCallbacks.has(fn)) {
+    warnedCallbacks.add(fn);
+    console.warn(
+      `lambda() was called with an anonymous (inline) callback function. ` +
+        `This prevents deduplication. Define the callback at file scope or ` +
+        `give it a name for better optimization. ` +
+        `See: https://grafast.org/grafast/standard-steps/lambda#define-callback-in-top-scope`,
+    );
+  }
   const $in = multistep(spec);
   const $lambda = new LambdaStep<UnwrapMultistep<TInMultistep>, TOut>($in, fn);
   if ((fn as any).hasSideEffects) {

grafast/website/grafast/plan-resolvers/best-practices.md

@@ -0,0 +1,245 @@
+---
+sidebar_position: 5.5
+title: "Best practices"
+---
+
+# Best practices for plan resolvers
+
+Plan resolvers are **declarative**: they build a graph of steps at plan-time,
+and Gra*fast* executes that graph later in batches. Keeping this mental model in
+mind leads to cleaner, faster plans. This page collects the most important
+recommendations.
+
+## Extract arguments deeply
+
+When accessing nested argument values, prefer extracting the leaf value directly
+rather than extracting an intermediate object and then pulling values from it.
+This gives Gra*fast* more information about what you actually need, which
+enables better optimization.
+
+```graphql
+input UserFilter {
+  author: String
+  publishedAfter: Int
+}
+
+type Query {
+  bookCount(search: String, filter: UserFilter): Int!
+}
+```
+
+### Don't: shallow extraction then transform
+
+```ts
+function bookCount_plan($parent, fieldArgs) {
+  const $filter = fieldArgs.getRaw("filter");
+  // ✘ Creates an unnecessary intermediate lambda step
+  const $author = lambda($filter, (f) => f?.author);
+  // ...
+}
+```
+
+### Do: deep extraction directly
+
+```ts
+function bookCount_plan($parent, fieldArgs) {
+  // ✔ One step, directly optimizable
+  const $author = fieldArgs.getRaw(["filter", "author"]);
+  const $publishedAfter = fieldArgs.getRaw(["filter", "publishedAfter"]);
+  // ...
+}
+```
+
+You can also use the `$`-prefixed shortcut for the same result:
+
+```ts
+function bookCount_plan($parent, fieldArgs) {
+  const { $search, $filter } = fieldArgs;
+  const { $author, $publishedAfter } = $filter;
+  // ...
+}
+```
+
+Both `.getRaw()` with a path array and the `$`-prefixed destructuring give
+Gra*fast* direct visibility into exactly which leaf values you need, allowing it
+to skip unnecessary work and optimize the plan more aggressively.
+
+## Prefer custom steps over `lambda`
+
+[`lambda()`](../standard-steps/lambda.md) is an escape hatch — it
+processes values **one at a time** rather than in batches. This is fine for
+trivial synchronous transforms (string concatenation, simple math), but for
+anything more complex you should create a custom step class.
+
+### Why custom steps are better
+
+| | `lambda` | Custom step |
+|---|---|---|
+| Batching | No — called once per value | Yes — `execute()` receives the full batch |
+| Deduplication | Only if callback is the same reference | Full control via `deduplicate()` |
+| Optimization | None | Can implement `optimize()` |
+| Side effects | Not supported (use `sideEffect()`) | Full control via `hasSideEffects` |
+
+### When `lambda` is appropriate
+
+- Concatenating strings: `lambda([$first, $last], ([f, l]) => \`${f} ${l}\`, true)`
+- Simple math: `lambda($n, (n) => n + 1, true)`
+- Trivial data mapping that doesn't benefit from batching
+
+### When to create a custom step
+
+- The transform involves I/O or async work — use
+  [`loadOne()`](../standard-steps/loadOne.md) /
+  [`loadMany()`](../standard-steps/loadMany.md) instead
+- You want deduplication (e.g. multiple fields perform the same transform)
+- The logic is non-trivial or would benefit from batching
+
+### Example: custom step
+
+```ts
+import { UnbatchedStep } from "grafast";
+
+export class FullNameStep extends UnbatchedStep<string> {
+  static $$export = {
+    moduleName: "my-app",
+    exportName: "FullNameStep",
+  };
+  isSyncAndSafe = true;
+
+  constructor($firstName: ExecutableStep<string>, $lastName: ExecutableStep<string>) {
+    super();
+    this.addDependency($firstName);
+    this.addDependency($lastName);
+  }
+
+  // Steps with identical dependencies are candidates for deduplication
+  deduplicate(peers: FullNameStep[]): FullNameStep[] {
+    return peers;
+  }
+
+  unbatchedExecute(_extra: UnbatchedExecutionExtra, firstName: string, lastName: string) {
+    return `${firstName} ${lastName}`;
+  }
+}
+
+export function fullName($first: ExecutableStep<string>, $last: ExecutableStep<string>) {
+  return new FullNameStep($first, $last);
+}
+```
+
+## Define lambda callbacks at file scope
+
+If you do use `lambda`, **always define the callback at file/module scope**
+(or import it from another file) rather than inline. Gra*fast* deduplicates
+lambda steps by comparing the callback reference &mdash; inline arrow functions
+create a new reference on every call, defeating deduplication.
+
+### Don't: inline callback
+
+```ts
+const objects = {
+  User: {
+    plans: {
+      fullName($user) {
+        const $firstName = $user.get("firstName");
+        const $lastName = $user.get("lastName");
+        // ✘ New function reference every time — cannot be deduplicated
+        return lambda([$firstName, $lastName], ([f, l]) => `${f} ${l}`, true);
+      },
+    },
+  },
+};
+```
+
+### Do: file-scoped callback
+
+```ts
+// ✔ Defined once at module scope — same reference every time
+function fullname([firstName, lastName]: [string, string]): string {
+  return `${firstName} ${lastName}`;
+}
+
+const objects = {
+  User: {
+    plans: {
+      fullName($user) {
+        const $firstName = $user.get("firstName");
+        const $lastName = $user.get("lastName");
+        return lambda([$firstName, $lastName], fullname, true);
+      },
+    },
+  },
+};
+```
+
+## Don't use `try`/`catch` in plan resolvers
+
+Plan resolvers run at **plan-time**, not execution-time. They build a
+declarative graph of steps &mdash; think of them like React component render
+functions where steps are like hooks. Using `try`/`catch` introduces imperative
+control flow that doesn't fit this model.
+
+### Why it doesn't work
+
+- Plan resolvers don't execute your data-fetching logic &mdash; they only
+  **describe** it. A `try` block around step creation doesn't catch runtime
+  data errors because those errors happen later, during execution.
+- Wrapping step creation in `try`/`catch` can mask plan-time programming
+  errors that should be fixed, not caught.
+- It suggests a misunderstanding of the plan/execute separation.
+
+### Don't: try/catch around steps
+
+```ts
+// ✘ This try/catch is meaningless — runtime errors happen during execution,
+// not during planning
+function post_author_plan($post) {
+  try {
+    const $authorId = $post.get("authorId");
+    return loadOne($authorId, batchGetAuthorById);
+  } catch (e) {
+    return constant(null);
+  }
+}
+```
+
+### Do: use flow control steps
+
+Gra*fast* provides declarative flow control for handling errors and null values
+at execution-time:
+
+```ts
+import { loadOne, trap, inhibitOnNull, TRAP_ERROR } from "grafast";
+
+function post_author_plan($post) {
+  const $authorId = $post.get("authorId");
+
+  // Guard against null authorId — skip the load entirely
+  const $guardedId = inhibitOnNull($authorId);
+
+  // Load the author; if it errors, convert to null
+  const $author = loadOne($guardedId, batchGetAuthorById);
+  return trap($author, TRAP_ERROR);
+}
+```
+
+The key flow control steps are:
+
+- [`inhibitOnNull()`](../standard-steps/inhibitOnNull.mdx) &mdash; suppresses
+  downstream work when a value is `null`
+- [`assertNotNull()`](../standard-steps/assertNotNull.mdx) &mdash; turns
+  `null` into a `SafeError` visible to clients
+- [`trap()`](../standard-steps/trap.mdx) &mdash; recovers inhibited or errored
+  values back into ordinary data (e.g. `null` or an empty list)
+
+See [Thinking in plans: Flow control](../flow.mdx#flow-control) for more
+details on when and how to use these.
+
+## Summary
+
+| Recommendation | Why |
+|---|---|
+| [Extract arguments deeply](#extract-arguments-deeply) | Fewer intermediate steps, better optimization |
+| [Prefer custom steps over `lambda`](#prefer-custom-steps-over-lambda) | Batching, deduplication, optimization |
+| [File-scoped lambda callbacks](#define-lambda-callbacks-at-file-scope) | Enables deduplication |
+| [No `try`/`catch`](#dont-use-trycatch-in-plan-resolvers) | Plan resolvers are declarative; use flow control steps |

grafast/website/grafast/plan-resolvers/index.mdx

@@ -25,6 +25,15 @@ as steps to be populated at execution time for each request.
 
 :::
 
+:::tip[Best practices]
+
+For recommendations on writing efficient and correct plan resolvers —
+including deep argument extraction, when to use custom steps vs `lambda`,
+and how to handle errors declaratively — see the
+[Best practices](./best-practices.md) guide.
+
+:::
+
 ## Field plan resolvers
 
 ```ts

grafast/website/grafast/standard-steps/lambda.md

@@ -80,6 +80,22 @@ not do this unless you are certain!
 
 :::
 
+## When to use something else
+
+Before reaching for `lambda`, consider whether a better tool exists:
+
+- **I/O or async work** → use [`loadOne()`](./loadOne.md) or
+  [`loadMany()`](./loadMany.md) which support batching
+- **Non-trivial transforms that appear in multiple fields** → create a
+  [custom step class](../step-classes.mdx) with `deduplicate()` support
+- **Side effects** → use [`sideEffect()`](/grafast/standard-steps/sideEffect)
+
+`lambda` is best reserved for trivial, synchronous, pure transforms such as
+string concatenation or simple arithmetic.
+
+See [Plan resolver best practices](../plan-resolvers/best-practices.md) for
+more guidance.
+
 ## Warning: no batching!
 
 **`lambda` is an escape hatch** that breaks you out of Gra*fast*'s batching;