Repository navigation
Conversation
The only non-test use of graphql is the DocumentNode type in the HTTP adapter declarations, and the query REPL no longer loads graphql at runtime, so users do not need a copy of graphql installed by the SDK. The declaration build now inlines the graphql types under dist/vendor/graphql with its LICENSE, and keeps the emitted layout independent of where the package manager stores dependencies on the build machine.
…ctory The workspace package layout was cut at the first packages/ segment of the build path, so a checkout located under another packages/ directory emitted declarations outside dist/shared and dist/tailor-proto. The changeset now says when the missing graphql install affects users.
🦋 Changeset detectedLatest commit: bb8d271 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@tailor-platform/create-sdk
@tailor-platform/eslint-plugin-sdk
@tailor-platform/sdk
@tailor-platform/sdk-plugin-frontend
@tailor-platform/sdk-plugin-seed
@tailor-platform/sdk-plugin-setup
@tailor-platform/sdk-plugin-tailordb-erd
commit: |
This comment has been minimized.
This comment has been minimized.
| "@types/semver": "7.8.0", | ||
| "@vitest/coverage-v8": "5.0.5", | ||
| "eslint-plugin-zod": "5.2.0", | ||
| "graphql": "17.0.2", |
There was a problem hiding this comment.
Without graphql installed, the executor's operation.query (kind: "graphql") is no longer type-checked. Its type comes from @urql/core, whose document types rely on @0no-co/graphql.web's /*@ts-ignore*/ import * as GraphQL from 'graphql'; once graphql can't be resolved they accept any value. Until now the SDK's own graphql resolved that import.
createExecutor({
name: "bad",
trigger: scheduleTrigger({ cron: "0 0 * * *" }),
operation: { kind: "graphql", query: 1 },
});In an npm or pnpm project that installs the packed SDK without graphql, tsc --strict rejects this on the base (Type 'number' is not assignable to type 'DocumentInput<unknown, AnyVariables>') and accepts it on this PR. Typing query with the inlined DocumentNode, as the HTTP adapter does, would avoid relying on graphql being resolvable.
There was a problem hiding this comment.
Addressed by b460618: query is now typed string | DocumentNode with the bundled types, and @urql/core is removed.
With the packed SDK installed without graphql, your query: 1 example now fails tsc --strict with TS2769 (Type 'number' is not assignable to type 'string | DocumentNode').
A string query compiles cleanly, and the published .d.mts files have no references to graphql or @urql.
| unbundle: true, | ||
| root: "src", | ||
| deps: { neverBundle: externalDeps }, | ||
| deps: { neverBundle: externalDeps, dts: { alwaysBundle: [/^graphql(\/|$)/] } }, |
There was a problem hiding this comment.
The inlined types under dist/vendor/graphql aren't reachable through exports, so with declaration: true a project can no longer emit an inferred type that reaches them, even when it installs graphql itself:
declare const doc: HttpAdapterTypedDocumentNode<{ user: { id: string } }, { id: string }>;
export const definitions = doc.definitions;With graphql@17.0.2 installed via pnpm, this emits on the base and fails on this PR with TS2883 (The inferred type of 'definitions' cannot be named without a reference to 'DefinitionNode' from './node_modules/@tailor-platform/sdk/dist/vendor/graphql/language/ast.mjs').
There was a problem hiding this comment.
b460618 adds ./vendor/graphql/* as a types-only export.
With the packed SDK, your definitions example with declaration: true now emits import("@tailor-platform/sdk/vendor/graphql/language/ast").DefinitionNode[] instead of failing with TS2883.
| onSuccess: (config) => { | ||
| const vendorDir = path.join(config.outDir, "vendor/graphql"); | ||
| mkdirSync(vendorDir, { recursive: true }); | ||
| copyToOutDir(config.outDir, "node_modules/graphql/LICENSE", "vendor/graphql/LICENSE"); |
There was a problem hiding this comment.
node_modules/graphql/ is repeated here and in the //#region regex below instead of reusing GRAPHQL_DIR, and vendor/graphql appears here, on L26 and on L89. Sharing constants for both, and deriving the LICENSE destination from vendorDir, would keep them in sync.
| ]; | ||
|
|
||
| const GRAPHQL_DIR = "node_modules/graphql/"; | ||
| const WORKSPACE_DIR = /^(?:.*\/)?packages\//; |
There was a problem hiding this comment.
Next to GRAPHQL_DIR, WORKSPACE_DIR reads like a directory path, but it's a pattern for everything up to the last packages/. A name like UP_TO_PACKAGES_DIR would say what it strips.
Type the executor query option with the bundled DocumentNode instead of the type taken from @urql/core, so it is checked without graphql installed, and drop @urql/core and its wonka dependency. Publish the bundled declarations under a types-only ./vendor/graphql/* export so declaration output that mentions a DocumentNode type resolves for consumers. Share the vendor directory constants in tsdown.config.ts and rename WORKSPACE_DIR to UP_TO_PACKAGES_DIR. Skip wildcard exports in the vitest alias generation and the zod isolation and public API JSDoc checks.
Resolve the packages/sdk/package.json conflicts by keeping the removal of @urql/core and the graphql devDependency from this branch together with the amaro, eslint-plugin-zod, oxfmt, oxlint and oxlint-tsgolint updates from the base branch. Regenerate pnpm-lock.yaml.
Code Metrics Report (packages/sdk)Type instantiations changed in 8 cases
executor-baselinecreateExecutor({
name: "baseline",
description: "Baseline executor",
trigger: incomingWebhookTrigger(),
operation: {
kind: "function",
body: async () => {
console.log("Baseline executed");
},
},
})executor-schedulereturn createExecutor({
name: "executor0",
description: "Scheduled executor 0",
trigger: scheduleTrigger({ cron: "0 0 * * *" }),
operation: {
kind: "graphql",
appName: "my-app",
query: `mutation { log(message: "executed") }`,
variables: () => ({}),
},
});executor-webhookreturn createExecutor({
name: "executor0",
description: "Webhook executor 0",
trigger: incomingWebhookTrigger(),
operation: {
kind: "graphql",
appName: "my-app",
query: `mutation { process(data: $data) }`,
variables: () => ({ data: "webhook-data" }),
},
});executor-record-createdreturn createExecutor({
name: "executor0",
description: "Record created trigger executor 0",
trigger: recordCreatedTrigger({ type: dummyType }),
operation: {
kind: "graphql",
appName: "my-app",
query: `mutation { log(message: "executed") }`,
variables: () => ({}),
},
});executor-record-updatedreturn createExecutor({
name: "executor0",
description: "Record updated trigger executor 0",
trigger: recordUpdatedTrigger({ type: dummyType }),
operation: {
kind: "graphql",
appName: "my-app",
query: `mutation { log(message: "executed") }`,
variables: () => ({}),
},
});executor-record-deletedreturn createExecutor({
name: "executor0",
description: "Record deleted trigger executor 0",
trigger: recordDeletedTrigger({ type: dummyType }),
operation: {
kind: "graphql",
appName: "my-app",
query: `mutation { log(message: "executed") }`,
variables: () => ({}),
},
});executor-resolverreturn createExecutor({
name: "executor0",
description: "Resolver executed trigger executor 0",
trigger: resolverExecutedTrigger({ resolver: dummyResolver }),
operation: {
kind: "graphql",
appName: "my-app",
query: `mutation { log(message: "executed") }`,
variables: () => ({}),
},
});executor-operation-gqlconst query = `
query GetUser($id: ID!) {
user(id: $id) {
id
name
}
}
`;
return createExecutor({
name: "executor0",
description: "GQL operation executor 0",
trigger: incomingWebhookTrigger(),
operation: {
kind: "graphql",
query,
variables: () => ({ id: "1" }),
},
});Details | | main (0c17d81) | #2816 (527bbed) | +/- |
|--------------------|----------------|-----------------|-------|
- | Coverage | 89.2% | 89.1% | -0.1% |
| Files | 578 | 578 | 0 |
| Lines | 25815 | 25625 | -190 |
- | Covered | 23035 | 22853 | -182 |
+ | Code to Test Ratio | 1:0.5 | 1:0.5 | +0.0 |
| Code | 212577 | 212627 | +50 |
+ | Test | 120407 | 120458 | +51 |Code coverage of files in pull request scope (96.5% → 100.0%)
SDK Configure Bundle Size
Runtime Performance
Type Performance (instantiations)
Reported by octocov |
Summary
graphql(17.0.2) fromdependenciestodevDependenciesof@tailor-platform/sdk, so the SDK no longer installs it into users'node_modulesand users no longer end up with twographqlversions side by side.DocumentNodetypes that the HTTP adapter and the executorqueryoption use into the published declarations underdist/vendor/graphql, with thegraphqlLICENSE next to them, and expose them through a types-only./vendor/graphql/*export so thatdeclarationoutput that mentions aDocumentNodetype resolves for consumers.queryoption asstring | DocumentNode(the bundled type) instead of the type taken from@urql/core, whosegraphqlimport resolves toanywhengraphqlis not installed.@urql/coreand itswonkadependency are no longer used and are removed.dist/vendor/graphql,dist/shared,dist/tailor-proto) instead of paths that mirror where the package manager stores dependencies, and the//#regioncomments no longer contain that store path.graphqlis used at runtime nowhere in the SDK: the HTTP adapter bundle and the query REPL use@0no-co/graphql.web. The only other use isimport type { DocumentNode } from "graphql"in the HTTP adapter and executor type declarations.Dependencies
feat/enlumine,refactor(cli): highlight query REPL input with enlumine, 748a0d6). This PR's base is that branch, so the diff shows only this change. That commit removes the last runtime use ofgraphql, so this must not be merged before it.Compatibility
I packed the tarball and type-checked consumer projects with
tsc --strictand noskipLibCheck, unless noted:graphqlinstalled:createHttpAdapterwith aHttpAdapterTypedDocumentNodetype-checks, mismatchedvariablesare rejected, andDocumentNodeis notany. In an executor,query: 1is a type error andquery: "query { a }"type-checks.declaration: truewithexport const definitions = doc.definitions(from aHttpAdapterTypedDocumentNode) compiles withoutgraphqlinstalled and emitsreadonly import("@tailor-platform/sdk/vendor/graphql/language/ast").DefinitionNode[], which resolves through the new./vendor/graphql/*export. Without that export it failed with TS2742.graphql16.11.0 and 17.0.2:DocumentNode,TypedQueryDocumentNodeandTypedDocumentNodefrom@graphql-typed-document-node/coreare accepted by HTTP adapterinputhandlers, andresp.datais inferred. Aparse()result is accepted by the executorqueryoption.graphql15.9.0 the documents are rejected by HTTP adapters, which is also the case on 2.36.0 because the types resolve to the SDK's owngraphql17. For the executorqueryoption, aparse()result is now rejected withskipLibCheck: true; withoutskipLibCheckthe@0no-co/graphql.webdeclarations already fail to compile withgraphql15.graphql16.11.0 and the SDK with npm: on 2.36.0node_modules/@tailor-platform/sdk/node_modules/graphql(17.0.2),@urql/coreandwonkaare installed; with this change none of them is.graphqlwithout declaring it fails to resolve it with pnpm on 2.36.0 already; with npm it resolved through hoisting and stops resolving after this change (ERR_MODULE_NOT_FOUND). The changeset ispatchand tells users to declaregraphqlthemselves.DocumentNodeto the executorqueryoption type-checks but is stringified to[object Object]by the executor schema on 2.36.0 already; this change does not alter it.