From 0048c94c7ccdcfa5e62687446376ce8341c002b5 Mon Sep 17 00:00:00 2001 From: Jack Williams Date: Mon, 26 Feb 2024 11:49:26 +0000 Subject: [PATCH] Test for `composite: true` errors in CI, ensuring we can't ship those issues (#501) ## Summary We've seen some issues crop up when `composite: true` is present in `tsconfig.json` files. This PR adds a `composite: true` project where we test the Inngest package for compliance for the types that it exports. We purposefully want to limit the number of types exported from the main `"inngest"` entrypoint, as each type exported here becomes part of the public API, where changing those types is a breaking change. Therefore, the composite check gives us a (albeit inaccurate) test that can stop us shipping these dangerous changes. ## Checklist - [ ] ~Added a [docs PR](https://github.com/inngest/website) that references this PR~ N/A Bug fix and testing - [x] Added unit/integration tests - [x] Added changesets if applicable ## Related - #384 - #385 - #437 - #460 --- .changeset/three-worms-hammer.md | 5 ++ .github/workflows/pr.yml | 11 ++++ packages/inngest/.eslintrc.js | 2 +- packages/inngest/etc/inngest.api.md | 28 ++++++++- packages/inngest/package.json | 1 + packages/inngest/src/index.ts | 3 + packages/inngest/src/types.ts | 14 +++++ .../inngest/test/composite_project/.gitignore | 4 ++ .../inngest/test/composite_project/README.md | 36 +++++++++++ .../test/composite_project/package.json | 18 ++++++ .../test/composite_project/src/index.ts | 62 +++++++++++++++++++ .../test/composite_project/tsconfig.json | 37 +++++++++++ 12 files changed, 218 insertions(+), 3 deletions(-) create mode 100644 .changeset/three-worms-hammer.md create mode 100644 packages/inngest/test/composite_project/.gitignore create mode 100644 packages/inngest/test/composite_project/README.md create mode 100644 packages/inngest/test/composite_project/package.json create mode 100644 packages/inngest/test/composite_project/src/index.ts create mode 100644 packages/inngest/test/composite_project/tsconfig.json diff --git a/.changeset/three-worms-hammer.md b/.changeset/three-worms-hammer.md new file mode 100644 index 00000000..64b8b905 --- /dev/null +++ b/.changeset/three-worms-hammer.md @@ -0,0 +1,5 @@ +--- +"inngest": patch +--- + +Fix failures for `composite: true` errors diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 0c338223..8a80ed3e 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -108,6 +108,17 @@ jobs: - uses: ./.github/actions/setup-and-build - run: pnpm run test:deps + inngest_test_composite: + name: "inngest: Composite tests" + runs-on: ubuntu-latest + defaults: + run: + working-directory: packages/inngest + steps: + - uses: actions/checkout@v3 + - uses: ./.github/actions/setup-and-build + - run: pnpm run test:composite + "eslint-plugin_test": name: "eslint-plugin: Test" runs-on: ubuntu-latest diff --git a/packages/inngest/.eslintrc.js b/packages/inngest/.eslintrc.js index 4285ebcb..e7959f1b 100644 --- a/packages/inngest/.eslintrc.js +++ b/packages/inngest/.eslintrc.js @@ -13,7 +13,7 @@ module.exports = { }, plugins: ["@typescript-eslint", "@inngest/internal", "import"], root: true, - ignorePatterns: ["dist/", "*.d.ts", "*.js"], + ignorePatterns: ["dist/", "*.d.ts", "*.js", "test/"], rules: { "prettier/prettier": "warn", "@inngest/internal/process-warn": "off", diff --git a/packages/inngest/etc/inngest.api.md b/packages/inngest/etc/inngest.api.md index b731390f..efd23104 100644 --- a/packages/inngest/etc/inngest.api.md +++ b/packages/inngest/etc/inngest.api.md @@ -19,6 +19,18 @@ export type AssertInternalEventPayloads; }; +// @public +export type BaseContext & string> = { + event: WithInvocation[TTrigger]>; + events: [ + EventsFromOpts[TTrigger], + ...EventsFromOpts[TTrigger][] + ]; + runId: string; + step: ReturnType, TTrigger>>; + attempt: number; +}; + // @public export interface ClientOptions { baseUrl?: string; @@ -44,7 +56,6 @@ export type ClientOptionsFromInngest> = TInngest e // @public export type Combine, TInc extends StandardEventSchemas> = IsStringLiteral extends true ? Simplify> & StandardEventSchemaToPayload> : StandardEventSchemaToPayload; -// Warning: (ae-forgotten-export) The symbol "BaseContext" needs to be exported by the entry point index.d.ts // Warning: (ae-internal-missing-underscore) The name "Context" should be prefixed with an underscore because the declaration is marked as @internal // // @internal @@ -455,6 +466,11 @@ export class NonRetriableError extends Error { readonly cause?: unknown; } +// Warning: (ae-forgotten-export) The symbol "HashedOp" needs to be exported by the entry point index.d.ts +// +// @public +export type OutgoingOp = Pick; + // @public export class ProxyLogger implements Logger { constructor(logger: Logger); @@ -512,6 +528,11 @@ export class RetryAfterError extends Error { readonly retryAfter: string; } +// @public +export type SendEventBaseOutput = { + ids: SendEventResponse["ids"]; +}; + // @public export interface ServeHandlerOptions extends RegisterOptions { client: Inngest.Any; @@ -595,7 +616,10 @@ export type ZodEventSchemas = Record; + +/** + * The shape of a step operation that is sent to an Inngest Server from an SDK. + * + * @public + */ export type OutgoingOp = Pick< HashedOp, "id" | "op" | "name" | "opts" | "data" | "error" | "displayName" @@ -251,6 +257,12 @@ export type WithInvocation = Simplify< { name: T["name"] | `${internalEvents.FunctionInvoked}` } & Omit >; +/** + * Base context object, omitting any extras that may be added by middleware or + * function configuration. + * + * @public + */ export type BaseContext< TOpts extends ClientOptions, TTrigger extends keyof EventsFromOpts & string, @@ -430,6 +442,8 @@ export type SendEventResponse = z.output; /** * The response in code from sending an event to Inngest. + * + * @public */ export type SendEventBaseOutput = { ids: SendEventResponse["ids"]; diff --git a/packages/inngest/test/composite_project/.gitignore b/packages/inngest/test/composite_project/.gitignore new file mode 100644 index 00000000..beb6593d --- /dev/null +++ b/packages/inngest/test/composite_project/.gitignore @@ -0,0 +1,4 @@ +node_modules +dist +tsconfig.tsbuildinfo +package-lock.json diff --git a/packages/inngest/test/composite_project/README.md b/packages/inngest/test/composite_project/README.md new file mode 100644 index 00000000..9f116f09 --- /dev/null +++ b/packages/inngest/test/composite_project/README.md @@ -0,0 +1,36 @@ +# Composite test project + +## What is this? + +This is a small project that uses `composite: true` in a `tsconfig.json` file. + +Projects with this setting require that imported packages directly export all +types that are needed for inference from the entrypoint. + +Every type we export is also part of the public API of a project; breaking +changes to those types are breaking changes for the package, too. Having a large +number of internal types exported at the entrypoint can make it very difficult +to avoid bumping the major version number. + +## How do I use it? + +You should test against this package by running `pnpm run test:composite` in +`packages/inngest`. This will: +- Build and pack `inngest` +- Install the packaged `inngest` into the composite project +- Attempt to build the project using `tsc` + +Any inference errors will appear as a TS2742 error, like so: +``` +src/index.ts:3:14 - error TS2742: The inferred type of 'inngest' cannot be named without a reference to '../node_modules/inngest/types'. This is likely not portable. A type annotation is necessary. +``` + +## Do I need to update it? + +The main tests this project is performing is that types are correctly inferred +by the various functions exported by the `"inngest"`. Therefore, if adding new +API methods and calls, it's ideal to add them into this project. + +This code is only built and is never run, so don't worry about it actually +working. + diff --git a/packages/inngest/test/composite_project/package.json b/packages/inngest/test/composite_project/package.json new file mode 100644 index 00000000..9c3671e8 --- /dev/null +++ b/packages/inngest/test/composite_project/package.json @@ -0,0 +1,18 @@ +{ + "name": "t_composite_test", + "version": "1.0.0", + "description": "", + "main": "index.js", + "scripts": { + "test": "tsc --build --force" + }, + "keywords": [], + "author": "", + "license": "ISC", + "devDependencies": { + "typescript": "^5.3.3" + }, + "dependencies": { + "inngest": "file:../../inngest.tgz" + } +} diff --git a/packages/inngest/test/composite_project/src/index.ts b/packages/inngest/test/composite_project/src/index.ts new file mode 100644 index 00000000..c7396717 --- /dev/null +++ b/packages/inngest/test/composite_project/src/index.ts @@ -0,0 +1,62 @@ +import * as Inngest from "inngest"; + +export const inngest = new Inngest.Inngest({ + id: "me", + schemas: new Inngest.EventSchemas().fromRecord<{ + foo: { data: { foo: string } }; + bar: { data: { bar: string } }; + }>(), + middleware: [ + new Inngest.InngestMiddleware({ + name: "foo", + init() { + return { + onFunctionRun(ctx) { + console.log(ctx); + + return { + transformInput(ctx) { + console.log(ctx); + }, + afterExecution() { + console.log("afterExecution"); + }, + afterMemoization() { + console.log("afterMemoization"); + }, + beforeExecution() { + console.log("beforeExecution"); + }, + beforeMemoization() { + console.log("beforeMemoization"); + }, + beforeResponse() { + console.log("beforeResponse"); + }, + transformOutput(ctx) { + console.log(ctx); + }, + }; + }, + onSendEvent() { + return { + transformInput(ctx) { + console.log(ctx); + }, + transformOutput(ctx) { + console.log(ctx); + }, + }; + }, + }; + }, + }), + ], +}); + +void inngest.send({ name: "foo", data: { foo: "bar" } }); + +inngest.createFunction({ id: "my-fn" }, { event: "foo" }, async (ctx) => { + console.log(ctx); + return { foo: "bar" }; +}); diff --git a/packages/inngest/test/composite_project/tsconfig.json b/packages/inngest/test/composite_project/tsconfig.json new file mode 100644 index 00000000..63d8a55a --- /dev/null +++ b/packages/inngest/test/composite_project/tsconfig.json @@ -0,0 +1,37 @@ +{ + "compilerOptions": { + "baseUrl": ".", + "target": "es5", + "forceConsistentCasingInFileNames": true, + "composite": true, + "lib": ["dom", "dom.iterable", "esnext"], + "allowJs": true, + "skipLibCheck": false, + "strict": true, + "noEmit": false, + "outDir": "dist", + "esModuleInterop": true, + "module": "esnext", + "moduleResolution": "bundler", + "resolveJsonModule": true, + "isolatedModules": true, + "jsx": "preserve", + "incremental": true, + "typeRoots": ["./types"], + "plugins": [ + { + "name": "next", + }, + ], + "paths": { + "~/*": ["./src/*"], + }, + }, + "include": [ + "next-env.d.ts", + "src/**/*.ts", + "**/*.tsx", + ".next/types/**/*.ts", + ], + "exclude": ["node_modules"], +}