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

Mapped object type returns from step.run() are oversimplified #98

Closed
jpwilliams opened this issue Feb 13, 2023 · 2 comments · Fixed by #512
Closed

Mapped object type returns from step.run() are oversimplified #98

jpwilliams opened this issue Feb 13, 2023 · 2 comments · Fixed by #512
Assignees
Labels
⬆️ improvement Performance, reliability, or usability improvements 📦 inngest Affects the `inngest` package

Comments

@jpwilliams
Copy link
Member

Describe the bug

Mapped object types with property overrides lose those overrides when output from step.run().

This affects a user using the plaid/plaid-node package, where many of the types returned from client methods are these mapped object types with overrides.

It looks like this project uses OpenAPITools/openapi-generator to create these types, so it's likely that any project using that will suffer the same fate.

To Reproduce

// Create a mapped object type with overrides
interface Foo {
  [x: string]: any;
  foo: boolean;
}

// Within a step function, return the object type
const result = await step.run("Issue", () => undefined as unknown as Foo);

// Observe that `result` has lost the `foo: boolean;` property.

Expected behavior

The object type should be maintained.

System info (please complete the following information):

  • OS: any
  • npm package Version: 1.0.0
  • Framework: any
  • Platform: any

Additional context

The issue is caused because we use a Jsonify<> type from sindresorhus/type-fest to simulate the (de)serialization of data as it moves to/from Inngest, which is stripping the extra properties.

Need to investigate whether this is a conscious choice and, if not, whether the behaviour of the object type being maintained is reasonable.

@jpwilliams jpwilliams added the Bug Something isn't working label Feb 13, 2023
@jpwilliams jpwilliams self-assigned this Feb 13, 2023
@JustinAimiable
Copy link

JustinAimiable commented Oct 19, 2023

👋 I wanted to see if there's been any progress or new info on this issue. In our codebase, we're exporting the handler callbacks that we pass into inngest.createFunction and testing them by passing a mock step object. This is a workaround to not being able to directly call the inngest functions that we create in order to unit test our handlers. One issue with this is the return types of step.run, which I've resorted to mocking like so:

run: (_name: string, cb: () => Promise<any>) => {
        return cb()
      }

Unfortunately, we lose type-safety on the return values from running a step this way. I tried using the built-in return type, but I think I'm running into the problem described in this issue when I do so. Any advice on how I can achieve type-safety and unit test my inngest functions? Thanks in advance :)

@jpwilliams jpwilliams added 📦 inngest Affects the `inngest` package ⬆️ improvement Performance, reliability, or usability improvements and removed Bug Something isn't working labels Nov 17, 2023
jpwilliams added a commit that referenced this issue Mar 7, 2024
@jpwilliams
Copy link
Member Author

@transitive-bullshit @JustinAimiable The fix for this is now shipped in v3.15.5. 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⬆️ improvement Performance, reliability, or usability improvements 📦 inngest Affects the `inngest` package
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants