From 8f03159f0ff0b0631707fc3224b597150d4226ef Mon Sep 17 00:00:00 2001 From: Jack Williams Date: Thu, 7 Mar 2024 12:55:36 +0000 Subject: [PATCH] Fix `Jsonify` issues, including preserving `unknown` types (#512) ## Summary `Jsonify` is used to represent a step output going through serialization as it passes to and from an Inngest Server in JSON. This used to be part of the `type-fest` package, but we removed that when introducing support for TypeScript 5.4 in #500, as our version support range for TS exceeded what was provided by the `type-fest` package. As part of that, we've pulled `Jsonify` into our code. There are some issues (in the related section) we can now fix much more easily. The issue that motivated this change (#509) would also have been solved by allowing a developer to provide types that describe how the output of a step is transformed. This is available, but is a rare ask and needs time to implement. ## Checklist - [ ] ~Added a [docs PR](https://github.com/inngest/website) that references this PR~ N/A Docs only outline that this is a helper, not specific behaviour - [x] Added unit/integration tests - [x] Added changesets if applicable ## Related - Fixes #509 (marking this as a fix, but again, note that middleware typing can also solve different strains of this issue) - Fixes #456 - Fixes #513 - Fixes #98 --- .changeset/fair-foxes-impress.md | 5 + .changeset/modern-crabs-enjoy.md | 5 + .changeset/purple-ducks-protect.md | 5 + packages/inngest/etc/inngest.api.md | 7 +- packages/inngest/package.json | 1 - .../src/components/EventSchemas.test.ts | 4 +- .../inngest/src/components/Inngest.test.ts | 5 +- packages/inngest/src/components/Inngest.ts | 2 +- .../src/components/InngestFunction.test.ts | 4 +- .../src/components/InngestMiddleware.test.ts | 12 +- .../src/components/InngestStepTools.test.ts | 3 +- .../components/execution/InngestExecution.ts | 3 +- .../inngest/src/components/execution/v0.ts | 3 +- packages/inngest/src/helpers/jsonify.test.ts | 105 ++++++++++++++++++ packages/inngest/src/helpers/jsonify.ts | 87 +++++++++------ packages/inngest/src/helpers/promises.test.ts | 2 +- packages/inngest/src/helpers/types.test.ts | 4 +- packages/inngest/src/helpers/types.ts | 89 ++++++++++++++- packages/inngest/src/test/helpers.ts | 6 + pnpm-lock.yaml | 3 - 20 files changed, 292 insertions(+), 63 deletions(-) create mode 100644 .changeset/fair-foxes-impress.md create mode 100644 .changeset/modern-crabs-enjoy.md create mode 100644 .changeset/purple-ducks-protect.md create mode 100644 packages/inngest/src/helpers/jsonify.test.ts diff --git a/.changeset/fair-foxes-impress.md b/.changeset/fair-foxes-impress.md new file mode 100644 index 00000000..0d469790 --- /dev/null +++ b/.changeset/fair-foxes-impress.md @@ -0,0 +1,5 @@ +--- +"inngest": patch +--- + +Fix union step outputs sometimes being typed as `any` diff --git a/.changeset/modern-crabs-enjoy.md b/.changeset/modern-crabs-enjoy.md new file mode 100644 index 00000000..42ef6383 --- /dev/null +++ b/.changeset/modern-crabs-enjoy.md @@ -0,0 +1,5 @@ +--- +"inngest": patch +--- + +Fix step output typing (`Jsonify`) removing detail from mapped object types with overrides diff --git a/.changeset/purple-ducks-protect.md b/.changeset/purple-ducks-protect.md new file mode 100644 index 00000000..0819713a --- /dev/null +++ b/.changeset/purple-ducks-protect.md @@ -0,0 +1,5 @@ +--- +"inngest": patch +--- + +Fix step output typing (`Jsonify`) omitting `unknown` and literals diff --git a/packages/inngest/etc/inngest.api.md b/packages/inngest/etc/inngest.api.md index fc75a7c6..122ec82f 100644 --- a/packages/inngest/etc/inngest.api.md +++ b/packages/inngest/etc/inngest.api.md @@ -4,9 +4,6 @@ ```ts -import { IsAny } from 'type-plus'; -import { IsEqual } from 'type-plus'; -import { IsNever } from 'type-plus'; import { z } from 'zod'; // Warning: (ae-forgotten-export) The symbol "Simplify" needs to be exported by the entry point index.d.ts @@ -383,9 +380,9 @@ export namespace InngestFunctionReference { }; export type HelperGenericArgs = HelperArgs | InngestFunction.Any; // Warning: (ae-forgotten-export) The symbol "PayloadFromAnyInngestFunction" needs to be exported by the entry point index.d.ts - // Warning: (ae-forgotten-export) The symbol "IsAny_2" needs to be exported by the entry point index.d.ts + // Warning: (ae-forgotten-export) The symbol "IsAny" needs to be exported by the entry point index.d.ts // Warning: (ae-forgotten-export) The symbol "ResolveSchema" needs to be exported by the entry point index.d.ts - export type HelperReturn = TArgs extends InngestFunction.Any ? InngestFunctionReference, GetFunctionOutput> : TArgs extends HelperArgs ? InngestFunctionReference> extends true ? MinimalEventPayload : Simplify> & Required>, "data">>>, ResolveSchema> : never; + export type HelperReturn = TArgs extends InngestFunction.Any ? InngestFunctionReference, GetFunctionOutput> : TArgs extends HelperArgs ? InngestFunctionReference> extends true ? MinimalEventPayload : Simplify> & Required>, "data">>>, ResolveSchema> : never; } // @public diff --git a/packages/inngest/package.json b/packages/inngest/package.json index d16ae9e4..52d587d1 100644 --- a/packages/inngest/package.json +++ b/packages/inngest/package.json @@ -180,7 +180,6 @@ "ms": "^2.1.3", "serialize-error-cjs": "^0.1.3", "strip-ansi": "^5.2.0", - "type-plus": "^5.1.0", "zod": "~3.22.3" }, "devDependencies": { diff --git a/packages/inngest/src/components/EventSchemas.test.ts b/packages/inngest/src/components/EventSchemas.test.ts index 2ec1979e..b47a22b5 100644 --- a/packages/inngest/src/components/EventSchemas.test.ts +++ b/packages/inngest/src/components/EventSchemas.test.ts @@ -1,10 +1,10 @@ import { EventSchemas } from "@local/components/EventSchemas"; import { Inngest, type GetEvents } from "@local/components/Inngest"; import { type internalEvents } from "@local/helpers/consts"; -import { type IsAny } from "@local/helpers/types"; +import { type IsAny, type IsEqual } from "@local/helpers/types"; import { type EventPayload, type FailureEventPayload } from "@local/types"; -import { assertType, type IsEqual } from "type-plus"; import { z } from "zod"; +import { assertType } from "../test/helpers"; // eslint-disable-next-line @typescript-eslint/no-explicit-any type Schemas> = GetEvents< diff --git a/packages/inngest/src/components/Inngest.test.ts b/packages/inngest/src/components/Inngest.test.ts index 976f13d9..e2781e8f 100644 --- a/packages/inngest/src/components/Inngest.test.ts +++ b/packages/inngest/src/components/Inngest.test.ts @@ -11,12 +11,11 @@ import { } from "@local"; import { type createStepTools } from "@local/components/InngestStepTools"; import { envKeys, headerKeys, internalEvents } from "@local/helpers/consts"; -import { type IsAny } from "@local/helpers/types"; +import { type IsAny, type IsEqual } from "@local/helpers/types"; import { type Logger } from "@local/middleware/logger"; import { type SendEventResponse } from "@local/types"; -import { assertType, type IsEqual } from "type-plus"; import { literal } from "zod"; -import { createClient } from "../test/helpers"; +import { assertType, createClient } from "../test/helpers"; const testEvent: EventPayload = { name: "test", diff --git a/packages/inngest/src/components/Inngest.ts b/packages/inngest/src/components/Inngest.ts index f0afda22..992ba0ef 100644 --- a/packages/inngest/src/components/Inngest.ts +++ b/packages/inngest/src/components/Inngest.ts @@ -1,4 +1,3 @@ -import { type IsNever } from "type-plus"; import { InngestApi } from "../api/api"; import { defaultDevServerHost, @@ -21,6 +20,7 @@ import { type Jsonify } from "../helpers/jsonify"; import { stringify } from "../helpers/strings"; import { type ExclusiveKeys, + type IsNever, type SendEventPayload, type SimplifyDeep, type WithoutInternal, diff --git a/packages/inngest/src/components/InngestFunction.test.ts b/packages/inngest/src/components/InngestFunction.test.ts index c144ec64..717a4215 100644 --- a/packages/inngest/src/components/InngestFunction.test.ts +++ b/packages/inngest/src/components/InngestFunction.test.ts @@ -28,6 +28,7 @@ import { OutgoingResultError, serializeError, } from "@local/helpers/errors"; +import { type IsEqual } from "@local/helpers/types"; import { DefaultLogger, ProxyLogger, @@ -40,8 +41,7 @@ import { type OutgoingOp, } from "@local/types"; import { fromPartial } from "@total-typescript/shoehorn"; -import { assertType, type IsEqual } from "type-plus"; -import { createClient, runFnWithStack } from "../test/helpers"; +import { assertType, createClient, runFnWithStack } from "../test/helpers"; type TestEvents = { foo: { data: { foo: string } }; diff --git a/packages/inngest/src/components/InngestMiddleware.test.ts b/packages/inngest/src/components/InngestMiddleware.test.ts index 70b97959..1f9b1202 100644 --- a/packages/inngest/src/components/InngestMiddleware.test.ts +++ b/packages/inngest/src/components/InngestMiddleware.test.ts @@ -1,13 +1,17 @@ /* eslint-disable @typescript-eslint/no-unsafe-assignment */ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { ExecutionVersion } from "@local/components/execution/InngestExecution"; import { Inngest } from "@local/components/Inngest"; import { referenceFunction } from "@local/components/InngestFunctionReference"; import { InngestMiddleware } from "@local/components/InngestMiddleware"; -import { type IsUnknown } from "@local/helpers/types"; +import { ExecutionVersion } from "@local/components/execution/InngestExecution"; +import { type IsEqual, type IsUnknown } from "@local/helpers/types"; import { StepOpCode } from "@local/types"; -import { assertType, type IsEqual } from "type-plus"; -import { createClient, runFnWithStack, testClientId } from "../test/helpers"; +import { + assertType, + createClient, + runFnWithStack, + testClientId, +} from "../test/helpers"; describe("stacking and inference", () => { describe("onFunctionRun", () => { diff --git a/packages/inngest/src/components/InngestStepTools.test.ts b/packages/inngest/src/components/InngestStepTools.test.ts index ec4ff7ee..bdbe2691 100644 --- a/packages/inngest/src/components/InngestStepTools.test.ts +++ b/packages/inngest/src/components/InngestStepTools.test.ts @@ -4,15 +4,16 @@ import { type EventsFromOpts } from "@local/components/Inngest"; import { InngestFunction } from "@local/components/InngestFunction"; import { referenceFunction } from "@local/components/InngestFunctionReference"; import { type createStepTools } from "@local/components/InngestStepTools"; +import { type IsEqual } from "@local/helpers/types"; import { StepOpCode, type ClientOptions, type InvocationResult, } from "@local/types"; import ms from "ms"; -import { assertType, type IsEqual } from "type-plus"; import { z } from "zod"; import { + assertType, createClient, getStepTools, testClientId, diff --git a/packages/inngest/src/components/execution/InngestExecution.ts b/packages/inngest/src/components/execution/InngestExecution.ts index 43f118e3..341429bf 100644 --- a/packages/inngest/src/components/execution/InngestExecution.ts +++ b/packages/inngest/src/components/execution/InngestExecution.ts @@ -1,8 +1,7 @@ import Debug, { type Debugger } from "debug"; -import { type MaybePromise } from "type-plus"; import { type ServerTiming } from "../../helpers/ServerTiming"; import { debugPrefix } from "../../helpers/consts"; -import { type Simplify } from "../../helpers/types"; +import { type MaybePromise, type Simplify } from "../../helpers/types"; import { type Context, type IncomingOp, type OutgoingOp } from "../../types"; import { type Inngest } from "../Inngest"; import { type ActionResponse } from "../InngestCommHandler"; diff --git a/packages/inngest/src/components/execution/v0.ts b/packages/inngest/src/components/execution/v0.ts index 103c46cb..3289a92e 100644 --- a/packages/inngest/src/components/execution/v0.ts +++ b/packages/inngest/src/components/execution/v0.ts @@ -1,6 +1,5 @@ import canonicalize from "canonicalize"; import { sha1 } from "hash.js"; -import { type MaybePromise } from "type-plus"; import { z } from "zod"; import { ErrCode, @@ -15,7 +14,7 @@ import { resolveNextTick, runAsPromise, } from "../../helpers/promises"; -import { type PartialK } from "../../helpers/types"; +import { type MaybePromise, type PartialK } from "../../helpers/types"; import { StepOpCode, failureEventErrorSchema, diff --git a/packages/inngest/src/helpers/jsonify.test.ts b/packages/inngest/src/helpers/jsonify.test.ts new file mode 100644 index 00000000..8cfec0f8 --- /dev/null +++ b/packages/inngest/src/helpers/jsonify.test.ts @@ -0,0 +1,105 @@ +/* eslint-disable @typescript-eslint/no-explicit-any */ +import { type Jsonify } from "@local/helpers/jsonify"; +import { type IsAny, type IsEqual, type IsUnknown } from "@local/helpers/types"; +import { assertType } from "../test/helpers"; + +describe("Jsonify", () => { + describe("unnested", () => { + test("allows `any`", () => { + type Actual = Jsonify; + assertType>(true); + }); + + test("allows `unknown`", () => { + type Actual = Jsonify; + assertType>(true); + }); + + test("allows number literals", () => { + type Actual = Jsonify<1>; + type Expected = 1; + assertType>(true); + }); + + test("allows string literals", () => { + type Actual = Jsonify<"foo">; + type Expected = "foo"; + assertType>(true); + }); + }); + + describe("nested", () => { + test("allows `any`", () => { + type Actual = Jsonify<{ foo: any }>; + type Expected = { foo: any }; + assertType>(true); + }); + + test("allows `unknown`", () => { + type Actual = Jsonify<{ foo: unknown }>; + type Expected = { foo: unknown }; + assertType>(true); + }); + + test("allows number literals", () => { + type Actual = Jsonify<{ foo: 1 }>; + type Expected = { foo: 1 }; + assertType>(true); + }); + + test("allows string literals", () => { + type Actual = Jsonify<{ foo: "bar" }>; + type Expected = { foo: "bar" }; + assertType>(true); + }); + }); + + describe("#513", () => { + test("appropriately types `string | null` when unnested", () => { + type Actual = Jsonify; + type Expected = string | null; + assertType>(false); + assertType>(true); + }); + + test("appropriately types `string | null` when nested", () => { + type Actual = Jsonify<{ foo: string | null }>; + type Expected = { foo: string | null }; + assertType>(false); + assertType>(true); + }); + }); + + describe("#98", () => { + test("allows mapped types with overrides when unnested", () => { + interface Foo { + [x: string]: any; + foo: boolean; + } + + type Actual = Jsonify; + type Expected = { + [x: string]: any; + foo: boolean; + }; + + assertType>(true); + assertType>(true); + assertType>(true); + }); + + test("allows mapped types with overrides when nested", () => { + interface Foo { + [x: string]: any; + foo: boolean; + } + + type Actual = Jsonify<{ foo: Foo }>; + type Expected = { foo: { [x: string]: any; foo: boolean } }; + + assertType>(true); + assertType>(true); + assertType>(true); + }); + }); +}); diff --git a/packages/inngest/src/helpers/jsonify.ts b/packages/inngest/src/helpers/jsonify.ts index 088793b0..25fb1847 100644 --- a/packages/inngest/src/helpers/jsonify.ts +++ b/packages/inngest/src/helpers/jsonify.ts @@ -14,8 +14,14 @@ /* eslint-disable @typescript-eslint/no-loss-of-precision */ /* eslint-disable @typescript-eslint/ban-types */ /* eslint-disable @typescript-eslint/no-explicit-any */ -import { type IsAny, type IsNever } from "type-plus"; -import { type IsUnknown, type Simplify } from "./types"; +import { + type IsAny, + type IsLiteral, + type IsNever, + type IsUnknown, + type KnownKeys, + type Simplify, +} from "./types"; // Note: The return value has to be `any` and not `unknown` so it can match `void`. type NotJsonable = ((...arguments_: any[]) => any) | undefined | symbol; @@ -159,11 +165,13 @@ type FilterDefinedKeys = Exclude< { [Key in keyof T]: IsAny extends true ? Key - : undefined extends T[Key] - ? never - : T[Key] extends undefined + : IsUnknown extends true + ? Key + : undefined extends T[Key] ? never - : BaseKeyFilter; + : T[Key] extends undefined + ? never + : BaseKeyFilter; }[keyof T], undefined >; @@ -268,30 +276,43 @@ const timeJson = JSON.parse(JSON.stringify(time)) as Jsonify; */ export type Jsonify = IsAny extends true ? any - : T extends PositiveInfinity | NegativeInfinity - ? null - : T extends JsonPrimitive - ? T - : // Any object with toJSON is special case - T extends { toJSON(): infer J } - ? (() => J) extends () => JsonValue // Is J assignable to JsonValue? - ? J // Then T is Jsonable and its Jsonable value is J - : Jsonify // Maybe if we look a level deeper we'll find a JsonValue - : // Instanced primitives are objects - T extends Number - ? number - : T extends String - ? string - : T extends Boolean - ? boolean - : T extends Map | Set - ? EmptyObject - : T extends TypedArray - ? Record - : T extends NotJsonable - ? never // Non-JSONable type union was found not empty - : T extends UnknownArray - ? JsonifyList - : T extends object - ? JsonifyObject> // JsonifyObject recursive call for its children - : never; // Otherwise any other non-object is removed + : IsUnknown extends true + ? unknown + : T extends PositiveInfinity | NegativeInfinity + ? null + : T extends JsonPrimitive + ? T + : // Any object with toJSON is special case + T extends { toJSON(): infer J } + ? (() => J) extends () => JsonValue // Is J assignable to JsonValue? + ? J // Then T is Jsonable and its Jsonable value is J + : Jsonify // Maybe if we look a level deeper we'll find a JsonValue + : // Instanced primitives are objects + T extends Number + ? number + : T extends String + ? string + : T extends Boolean + ? boolean + : T extends Map | Set + ? EmptyObject + : T extends TypedArray + ? Record + : T extends NotJsonable + ? never // Non-JSONable type union was found not empty + : T extends UnknownArray + ? JsonifyList + : T extends object + ? IsLiteral extends true + ? // JsonifyObject recursive call for its children + JsonifyObject> // An object with known keys can be processed directly + : Simplify< + JsonifyObject> & + // If the object has generic keys, this is a + // mapped type and we need to process the + // generic and known keys separately + JsonifyObject< + UndefinedToOptional>> + > + > + : never; // Otherwise any other non-object is removed diff --git a/packages/inngest/src/helpers/promises.test.ts b/packages/inngest/src/helpers/promises.test.ts index 4b4bfd65..5186c48a 100644 --- a/packages/inngest/src/helpers/promises.test.ts +++ b/packages/inngest/src/helpers/promises.test.ts @@ -1,5 +1,5 @@ import { runAsPromise } from "@local/helpers/promises"; -import { assertType } from "type-plus"; +import { assertType } from "../test/helpers"; describe("runAsPromise", () => { describe("synchronous functions", () => { diff --git a/packages/inngest/src/helpers/types.test.ts b/packages/inngest/src/helpers/types.test.ts index f44e2858..6f08f873 100644 --- a/packages/inngest/src/helpers/types.test.ts +++ b/packages/inngest/src/helpers/types.test.ts @@ -1,5 +1,5 @@ -import { type RecursiveTuple } from "@local/helpers/types"; -import { assertType, type IsEqual } from "type-plus"; +import { type IsEqual, type RecursiveTuple } from "@local/helpers/types"; +import { assertType } from "../test/helpers"; describe("RecursiveTuple", () => { test("should create a tuple of length 1", () => { diff --git a/packages/inngest/src/helpers/types.ts b/packages/inngest/src/helpers/types.ts index 34c93290..ea0d37b8 100644 --- a/packages/inngest/src/helpers/types.ts +++ b/packages/inngest/src/helpers/types.ts @@ -1,4 +1,3 @@ -import { type IsEqual } from "type-plus"; import { type EventPayload } from "../types"; /** @@ -346,3 +345,91 @@ export type IsUnknown = unknown extends T // `T` can be `unknown` or `any` ? true : false : false; + +/** +Returns a boolean for whether the two given types are equal. + +{@link https://github.com/microsoft/TypeScript/issues/27024#issuecomment-421529650} +{@link https://stackoverflow.com/questions/68961864/how-does-the-equals-work-in-typescript/68963796#68963796} + +Use-cases: +- If you want to make a conditional branch based on the result of a comparison of two types. + +@example +``` +import type {IsEqual} from 'type-fest'; + +// This type returns a boolean for whether the given array includes the given item. +// `IsEqual` is used to compare the given array at position 0 and the given item and then return true if they are equal. +type Includes = + Value extends readonly [Value[0], ...infer rest] + ? IsEqual extends true + ? true + : Includes + : false; +``` +*/ +export type IsEqual = (() => G extends A ? 1 : 2) extends < + G, +>() => G extends B ? 1 : 2 + ? true + : false; + +/** + * Returns a boolean for whether the given type `T` is `never`. + */ +export type IsNever = [T] extends [never] ? true : false; + +/** + * Given a type `T`, return `Then` if `T` is a string, number, or symbol + * literal, else `Else`. + * + * `Then` defaults to `true` and `Else` defaults to `false`. + * + * Useful for determining if an object is a generic type or has known keys. + * + * @example + * ```ts + * type IsLiteralType = IsLiteral<"foo">; // true + * type IsLiteralType = IsLiteral; // false + * + * type IsLiteralType = IsLiteral<1>; // true + * type IsLiteralType = IsLiteral; // false + * + * type IsLiteralType = IsLiteral; // true + * type IsLiteralType = IsLiteral; // false + * + * type T0 = { foo: string }; + * type HasAllKnownKeys = IsLiteral; // true + * + * type T1 = { [x: string]: any; foo: boolean }; + * type HasAllKnownKeys = IsLiteral; // false + * ``` + */ +export type IsLiteral = string extends T + ? Else + : number extends T + ? Else + : symbol extends T + ? Else + : Then; + +/** + * Given an object `T`, return the keys of that object that are known literals. + * + * Useful for filtering out generic mapped types from objects. + * + * @example + * ```ts + * type T0 = { foo: string }; + * type RegularKeys = keyof T0; // "foo" + * type KnownKeys = KnownLiteralKeys; // "foo" + * + * type T1 = { [x: string]: any; foo: boolean }; + * type RegularKeys = keyof T1; // string | number + * type KnownKeys = KnownLiteralKeys; // "foo" + * ``` + */ +export type KnownKeys = keyof { + [K in keyof T as IsLiteral]: T[K]; +}; diff --git a/packages/inngest/src/test/helpers.ts b/packages/inngest/src/test/helpers.ts index ecc32877..95474d09 100644 --- a/packages/inngest/src/test/helpers.ts +++ b/packages/inngest/src/test/helpers.ts @@ -1279,3 +1279,9 @@ export const checkIntrospection = ({ name, triggers }: CheckIntrospection) => { }); }); }; + +/** + * assert the subject satisfies the specified type T + * @type T the type to check against. + */ +export function assertType(subject: T): asserts subject is T {} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index e00df904..611bfa3a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -87,9 +87,6 @@ importers: strip-ansi: specifier: ^5.2.0 version: 5.2.0 - type-plus: - specifier: ^5.1.0 - version: 5.6.0 zod: specifier: ~3.22.3 version: 3.22.3