Skip to content

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

Open
msotnikov wants to merge 1 commit intographile:mainfrom
msotnikov:plan-resolvers-recommendations
Open

Extend documentation with a guide for plan resolvers (#244)#2985
msotnikov wants to merge 1 commit intographile:mainfrom
msotnikov:plan-resolvers-recommendations

Conversation

@msotnikov
Copy link

Description

Addresses graphile/crystal-pre-merge#244 — general recommendations when writing plan resolvers.

Documentation:

  • Added a new "Best practices" page (grafast/website/grafast/plan-resolvers/best-practices.md) covering the four recommendations from the issue:
    1. Deep argument extraction — use getRaw(["path", "to", "value"]) or $-prefixed destructuring instead of shallow extraction followed by lambda transforms
    2. Prefer custom steps over lambda — comparison table, guidance on when lambda is appropriate vs when to create a step class, worked example of a custom step
    3. File-scoped lambda callbacks — why inline anonymous functions defeat deduplication, with do/don't examples
    4. Avoid try/catch in plan resolvers — why imperative error handling doesn't fit the declarative model, with examples using inhibitOnNull() and trap() instead
  • Added a "When to use something else" section to lambda.md pointing users toward loadOne/loadMany, custom steps, and sideEffect() before they reach for lambda
  • Added a tip callout linking to the best practices page from the top of plan-resolvers/index.mdx

Runtime warning:

  • In dev mode (NODE_ENV=development), lambda() now emits a console.warn when called with an anonymous (unnamed) callback function, since such callbacks cannot be deduplicated. The warning fires once per
    unique callback reference (tracked via WeakSet).

Performance impact

No impact in production. In development mode, a single WeakSet lookup is added per lambda() call (negligible).

Security impact

None.

Checklist

  • My code matches the project's code style and yarn lint:fix passes.
  • I've added tests for the new feature, and yarn test passes.
  • I have detailed the new feature in the relevant documentation.
  • I have added this feature to 'Pending' in the RELEASE_NOTES.md file (if one exists).
  • If this is a breaking change I've explained why.

@github-project-automation github-project-automation bot moved this to 🌳 Triage in V5.0.0 Mar 9, 2026
@changeset-bot
Copy link

changeset-bot bot commented Mar 9, 2026

⚠️ No Changeset found

Latest commit: 393059c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🌳 Triage

Development

Successfully merging this pull request may close these issues.

1 participant