perf: speed up graphql commands#502
Conversation
|
Claude finished @rexxars's task in 2m 54s —— View job
ReviewIssue 1 (empty string Issue 2 —
const {createSchema} = await deps.resolveLocalPackage<typeof import('sanity')>('sanity', workDir)
const defaultSchema = createSchema({name: 'default', types: []})If Issue 3 — Multiple unnamed
Either detect and throw on duplicate names in // After building sources[], before return:
const names = sources.map((s) => s.name)
const hasDuplicates = names.length !== new Set(names).size
if (hasDuplicates) {
throw new Error('Multiple unstable_sources without unique names are not supported for graphql undeploy')
} |
d5ff00c to
b847785
Compare
Coverage Delta
Comparing 17 changed files against main @ Overall Coverage
|
804b55a to
a698c65
Compare
|
Claude is going to keep finding issue no matter how far I take this but I'm gonna call this readyish now. Will do a read-through and mark it ready. |
d4fc584 to
e757cb3
Compare
|
Claude finished @rexxars's task in 4m 41s —— View job Review
The three issues flagged in the previous review remain unaddressed after the refactor. Nothing new introduced in the latest commits. 1. Empty string still passes
|
219913c to
adc8006
Compare
| const mockStudioWorkerTask = vi.hoisted(() => vi.fn()) | ||
|
|
||
| vi.mock('node:worker_threads', () => ({ | ||
| isMainThread: true, |
There was a problem hiding this comment.
ah smart, good way to run worker tests 🤔
The getGraphQLAPIs worker (used by `graphql undeploy --api`) called getStudioWorkspaces() which runs resolveConfig() — compiling all schema types only to immediately discard them. Replace with a direct doImport() of the raw config to extract only workspace metadata (projectId, dataset, name), skipping resolveConfig() entirely. The worker still runs inside studioWorkerTask (Vite environment) for safe TS/browser handling — only the unnecessary schema compilation is removed. Remove SchemaError handling from getGraphQLAPIs and undeploy since schema errors cannot occur without schema compilation.
- Include original error message in non-SchemaError failures instead of the generic "Failed to resolve GraphQL APIs" - Re-throw when schema validation only contains warnings so the error isn't silently swallowed (previously exited with no APIs and no message) - Add tests for isSchemaError, warning-only schema errors, and error message preservation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the previously untested resolveGraphQLApiMetadata and resolveGraphQLApiMetadataFromConfig paths (lines 84-153), including single workspace fallback, multi-workspace config resolution, workspace not found, missing workspace name, empty config, dataset/projectId override vs fallback, and optional field preservation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Report all per-API errors instead of exiting on the first failure - Remove duplicate isStudioConfig from getGraphQLAPIs worker, import from cli-core - Add debug logging for non-SchemaError extraction failures - Add projectId/dataset assertion in resolveGraphQLApiMetadata - Add tests for per-API error branches and multi-API flag overrides Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Filter individual problems within configErrors groups to only include error-severity ones, preventing warning-level noise in output - Add source support to the metadata path so resolveGraphQLApiMetadata checks both workspace and source count, and resolves the `source` property from GraphQL API configs (matching the deploy path behavior) - Parallelize independent getCliConfig/findStudioConfigPath calls with Promise.all in both orchestrators - Remove process.chdir from integration test since functions accept cwd Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
resolveGraphQLAPIsFromConfig unconditionally overwrote apiDef.dataset and apiDef.projectId with source values, ignoring explicit overrides in the CLI graphql config. The metadata path already handled this correctly with nullish coalescing. Align the deploy path to match. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…orker - Remove unused `api` property from extractGraphQLAPIs worker payload and WorkerData interface (only `graphql` config is read) - Fix source metadata extraction to inherit projectId/dataset from the parent workspace when sources omit them (matching resolveConfig behavior), instead of silently skipping those sources - Remove dead typeof string guard in deploy.ts — already narrowed by the outer `if (apiDef.id)` check Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d deploys - Add missing %O format specifier for error arg in worker debug log - Fix --force --dry-run path: don't call spin.succeed() over the spin.warn() state already set by isResultValid(), and show the correct message with breaking changes instead of "no breaking changes" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When --api is specified, undeploy used process.cwd() to resolve the studio config, which fails if run from a subdirectory. Use this.getProjectRoot() to match deploy.ts behavior. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
These optional properties allow explicit overrides in the graphql config, taking precedence over the workspace/source configuration. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. deploy.ts: add debug() call inside SchemaError branch so schema
validation failures appear in debug traces
2. resolveGraphQLApisFromWorkspaces.ts: extract shared workspace/source
resolution into resolveWorkspaceAndSource() helper — eliminates ~50
lines of duplicated logic between the metadata and full-compile paths
3. isSchemaError.ts: add Array.isArray check on _validation to prevent
.map() from throwing if _validation is a non-array truthy value
4. getGraphQLAPIs.worker.ts: document the invariant that the workspace
name default ('default') must match resolveConfig()'s convention
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
GraphQLAPIConfig is the public interface for sanity.cli.ts graphql config. Dataset and projectId are resolved from the workspace/source configuration, not user-specified overrides. Use source values directly in the resolution functions instead of ?? fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- deploy.ts: print API name before per-API schema errors so multi-API debugging shows which API (dataset/tag) failed - extractGraphQLAPIs.worker.ts: handle Sanity's internal SchemaError (from createSchema()) in the per-API catch block via isSchemaError(), preserving structured problemGroups instead of degrading to a plain extractionError string - resolveGraphQLApisFromWorkspaces.ts: fix lodash-es barrel import lint error Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. deploy.ts: use schemaErrors?.length instead of truthy check to avoid treating an empty array as an error 2. extractGraphQLAPIs.worker.ts: guard against empty _validation on Sanity's internal SchemaError — falls through to extractionError with the error message instead of storing an empty schemaErrors array 3. getGraphQLAPIs.test.ts: add test for !isMainThread guard 4. isSchemaError.ts: narrow return type to assert _validation is a SchemaValidationProblemGroup[] array (intersected with Schema so validateSchema.worker.ts can still pass err.schema as Schema) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- extractGraphQLAPIs.worker.ts: only push schemaErrors when there are error-severity groups, matching the global path which re-throws warning-only errors. Previously, warning-only groups were stored as schemaErrors and blocked deployment with exit code 1. - deploy-errors.test.ts: replace overly broad regex with specific assertion for the SchemaError path. An invalid type reference triggers validation during resolveConfig(), not per-API extraction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The invalid schema test causes a stack overflow in CI (not a SchemaError), so the assertion must accept both error paths. Wrap process.chdir in try/finally to prevent cascade failures to subsequent tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace the unsafe process.chdir(testCwd) / process.chdir(cwd) pattern with testCommand's mocks.projectRoot option. The entire extractGraphQLAPIs chain is parameter-driven (uses workDir, not process.cwd()), so mocking getProjectRoot() is sufficient. This eliminates shared global state mutation and the need for try/finally cleanup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…e-level config SchemaError.print() now accepts an optional label so deploy.ts can pass the API-specific context directly, avoiding two consecutive warning lines. Also guard empty-string projectId/dataset on sources in both extractSourceMetadata (falls back to workspace defaults) and resolveGraphQLApiMetadata (throws a descriptive error). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
adc8006 to
af2ee4b
Compare
Description
The
graphql deploycommand currently loads studio config (via Vite), resolves workspaces, then sends serialized schema types back to the main thread — where it compiles schemas and extracts the GraphQL spec sequentially. This PR restructures the pipeline so that schema compilation and extraction happen inside the worker thread, avoiding the round-trip overhead of serializing full schema types across the thread boundary.Performance: ~15% faster on
graphql deploy(measured via wall-clock benchmarks againstbasic-studioandworst-case-studiofixtures). The gain comes from doing more work in the worker and sending back only the extracted GraphQL spec rather than raw schema types.Error handling: Schema errors (both from
createSchema()and fromextractFromSanitySchema()) are now caught per-API instead of failing the entire deployment. If one API has schema errors, the others still get processed and all errors are reported together.graphql undeploy: Now uses a lightweight metadata-only path (resolveGraphQLApiMetadata) that reads workspace config without compiling schemas at all — it only needs projectId/dataset/tag to identify which API to undeploy.What to review
extractGraphQLAPIs.ts/extractGraphQLAPIsWorker.ts— the new worker pipeline that does schema compilation + extraction in-thread. This is the core change.resolveGraphQLApisFromWorkspaces.ts— shared workspace/source resolution logic extracted from the old worker, now used by both the full-compile and metadata-only paths.deploy.ts— updated to consume pre-extracted GraphQL specs instead of compiling schemas itself. Per-API error handling forschemaErrorsandextractionError.getGraphQLAPIs.worker.ts— now servesgraphql undeploywith a lightweight metadata-only path (no schema compilation).Testing
extractGraphQLAPIsWorker(schema extraction, per-API error handling, Symbol serialization)resolveGraphQLApisFromWorkspaces(bothresolveGraphQLApisandresolveGraphQLApiMetadata— single/multi workspace, source inheritance, validation)SchemaError(custom labels, formatted output)graphqlAPIs.integration.test.ts) that runs the full extraction pipeline against real schema fixtures