Skip to content

Commit

Permalink
Fix Jsonify issues, including preserving unknown types (#512)
Browse files Browse the repository at this point in the history
## Summary
<!-- Succinctly describe your change, providing context, what you've
changed, and why. -->

`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
<!-- Tick these items off as you progress. -->
<!-- If an item isn't applicable, ideally please strikeout the item by
wrapping it in "~~"" and suffix it with "N/A My reason for skipping
this." -->
<!-- e.g. "- [ ] ~~Added tests~~ N/A Only touches docs" -->

- [ ] ~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
<!-- A space for any related links, issues, or PRs. -->
<!-- Linear issues are autolinked. -->
<!-- e.g. - INN-123 -->
<!-- GitHub issues/PRs can be linked using shorthand. -->
<!-- e.g. "- inngest/inngest#123" -->
<!-- Feel free to remove this section if there are no applicable related
links.-->
- 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
  • Loading branch information
jpwilliams committed Mar 7, 2024
1 parent 783e7be commit 8f03159
Show file tree
Hide file tree
Showing 20 changed files with 292 additions and 63 deletions.
5 changes: 5 additions & 0 deletions .changeset/fair-foxes-impress.md
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Fix union step outputs sometimes being typed as `any`
5 changes: 5 additions & 0 deletions .changeset/modern-crabs-enjoy.md
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Fix step output typing (`Jsonify`) removing detail from mapped object types with overrides
5 changes: 5 additions & 0 deletions .changeset/purple-ducks-protect.md
@@ -0,0 +1,5 @@
---
"inngest": patch
---

Fix step output typing (`Jsonify`) omitting `unknown` and literals
7 changes: 2 additions & 5 deletions packages/inngest/etc/inngest.api.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion packages/inngest/package.json
Expand Up @@ -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": {
Expand Down
4 changes: 2 additions & 2 deletions 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<T extends EventSchemas<any>> = GetEvents<
Expand Down
5 changes: 2 additions & 3 deletions packages/inngest/src/components/Inngest.test.ts
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion packages/inngest/src/components/Inngest.ts
@@ -1,4 +1,3 @@
import { type IsNever } from "type-plus";
import { InngestApi } from "../api/api";
import {
defaultDevServerHost,
Expand All @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions packages/inngest/src/components/InngestFunction.test.ts
Expand Up @@ -28,6 +28,7 @@ import {
OutgoingResultError,
serializeError,
} from "@local/helpers/errors";
import { type IsEqual } from "@local/helpers/types";
import {
DefaultLogger,
ProxyLogger,
Expand All @@ -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 } };
Expand Down
12 changes: 8 additions & 4 deletions 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", () => {
Expand Down
3 changes: 2 additions & 1 deletion packages/inngest/src/components/InngestStepTools.test.ts
Expand Up @@ -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,
Expand Down
@@ -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";
Expand Down
3 changes: 1 addition & 2 deletions 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,
Expand All @@ -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,
Expand Down
105 changes: 105 additions & 0 deletions 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<any>;
assertType<IsAny<Actual>>(true);
});

test("allows `unknown`", () => {
type Actual = Jsonify<unknown>;
assertType<IsUnknown<Actual>>(true);
});

test("allows number literals", () => {
type Actual = Jsonify<1>;
type Expected = 1;
assertType<IsEqual<Actual, Expected>>(true);
});

test("allows string literals", () => {
type Actual = Jsonify<"foo">;
type Expected = "foo";
assertType<IsEqual<Actual, Expected>>(true);
});
});

describe("nested", () => {
test("allows `any`", () => {
type Actual = Jsonify<{ foo: any }>;
type Expected = { foo: any };
assertType<IsEqual<Actual, Expected>>(true);
});

test("allows `unknown`", () => {
type Actual = Jsonify<{ foo: unknown }>;
type Expected = { foo: unknown };
assertType<IsEqual<Actual, Expected>>(true);
});

test("allows number literals", () => {
type Actual = Jsonify<{ foo: 1 }>;
type Expected = { foo: 1 };
assertType<IsEqual<Actual, Expected>>(true);
});

test("allows string literals", () => {
type Actual = Jsonify<{ foo: "bar" }>;
type Expected = { foo: "bar" };
assertType<IsEqual<Actual, Expected>>(true);
});
});

describe("#513", () => {
test("appropriately types `string | null` when unnested", () => {
type Actual = Jsonify<string | null>;
type Expected = string | null;
assertType<IsAny<Actual>>(false);
assertType<IsEqual<Actual, Expected>>(true);
});

test("appropriately types `string | null` when nested", () => {
type Actual = Jsonify<{ foo: string | null }>;
type Expected = { foo: string | null };
assertType<IsAny<Actual["foo"]>>(false);
assertType<IsEqual<Actual, Expected>>(true);
});
});

describe("#98", () => {
test("allows mapped types with overrides when unnested", () => {
interface Foo {
[x: string]: any;
foo: boolean;
}

type Actual = Jsonify<Foo>;
type Expected = {
[x: string]: any;
foo: boolean;
};

assertType<IsEqual<Actual, Expected>>(true);
assertType<IsEqual<Actual["foo"], boolean>>(true);
assertType<IsAny<Actual["bar"]>>(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<IsEqual<Actual, Expected>>(true);
assertType<IsEqual<Actual["foo"]["foo"], boolean>>(true);
assertType<IsAny<Actual["foo"]["bar"]>>(true);
});
});
});
87 changes: 54 additions & 33 deletions packages/inngest/src/helpers/jsonify.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -159,11 +165,13 @@ type FilterDefinedKeys<T extends object> = Exclude<
{
[Key in keyof T]: IsAny<T[Key]> extends true
? Key
: undefined extends T[Key]
? never
: T[Key] extends undefined
: IsUnknown<T[Key]> extends true
? Key
: undefined extends T[Key]
? never
: BaseKeyFilter<T, Key>;
: T[Key] extends undefined
? never
: BaseKeyFilter<T, Key>;
}[keyof T],
undefined
>;
Expand Down Expand Up @@ -268,30 +276,43 @@ const timeJson = JSON.parse(JSON.stringify(time)) as Jsonify<typeof time>;
*/
export type Jsonify<T> = IsAny<T> 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<J> // 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<any, any> | Set<any>
? EmptyObject
: T extends TypedArray
? Record<string, number>
: T extends NotJsonable
? never // Non-JSONable type union was found not empty
: T extends UnknownArray
? JsonifyList<T>
: T extends object
? JsonifyObject<UndefinedToOptional<T>> // JsonifyObject recursive call for its children
: never; // Otherwise any other non-object is removed
: IsUnknown<T> 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<J> // 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<any, any> | Set<any>
? EmptyObject
: T extends TypedArray
? Record<string, number>
: T extends NotJsonable
? never // Non-JSONable type union was found not empty
: T extends UnknownArray
? JsonifyList<T>
: T extends object
? IsLiteral<keyof T> extends true
? // JsonifyObject recursive call for its children
JsonifyObject<UndefinedToOptional<T>> // An object with known keys can be processed directly
: Simplify<
JsonifyObject<UndefinedToOptional<T>> &
// If the object has generic keys, this is a
// mapped type and we need to process the
// generic and known keys separately
JsonifyObject<
UndefinedToOptional<Pick<T, KnownKeys<T>>>
>
>
: never; // Otherwise any other non-object is removed
2 changes: 1 addition & 1 deletion 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", () => {
Expand Down
4 changes: 2 additions & 2 deletions 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", () => {
Expand Down

0 comments on commit 8f03159

Please sign in to comment.