Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strictNullChecks: false breaks Jsonify typing #534

Open
jpwilliams opened this issue Apr 9, 2024 · 1 comment
Open

strictNullChecks: false breaks Jsonify typing #534

jpwilliams opened this issue Apr 9, 2024 · 1 comment
Labels
Bug Something isn't working 📦 inngest Affects the `inngest` package

Comments

@jpwilliams
Copy link
Member

Summary

Step returns use the Jsonify type to represent the result of a step being (de)serialized as it is passed to and from Inngest.

If strictNullChecks: false is set in a user's tsconfig.json, however, this type sets some properties to optional that shouldn't be optional.

Example

Here's a playground example with strictNullChecks: false set. Notice how foo is optional where it shouldn't be. https://tsplay.dev/WyVMKw

import { Inngest } from "inngest";

const inngest = new Inngest({ id: "my-app" });

inngest.createFunction(
  { id: "my-fn" },
  { event: "my/event" },
  async ({ step }) => {
    // `foo` should not be optional
    const obj = await step.run("get-obj", () => ({
      //  ^? const obj: { foo?: string; }
      foo: "bar",
    }));
  },
);

Cause

This issue was introduced in #512 and released in 3.15.5. Specifically, this piece here:

? 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>>>
>
>

Solution

Some TypeScript packages that perform some similar functionality are very opinionated about required configuration options. For example, type-fest: sindresorhus/type-fest#668 (comment)

However, as we wish to fit as quietly into a user's existing codebase as possible, ideally we:

  • Add unit tests with a project using this option, which may surface other small issues
  • If we can, account for strictNullChecks: false in this UndefinedToOptional type
@jpwilliams jpwilliams added Bug Something isn't working 📦 inngest Affects the `inngest` package labels Apr 9, 2024
Copy link

linear bot commented Apr 9, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working 📦 inngest Affects the `inngest` package
Projects
None yet
Development

No branches or pull requests

1 participant