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

Infering the boundary callback arguments #2069

Closed
1 task
Tenga opened this issue Mar 6, 2024 · 10 comments · Fixed by #2101
Closed
1 task

Infering the boundary callback arguments #2069

Tenga opened this issue Mar 6, 2024 · 10 comments · Fixed by #2101
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@Tenga
Copy link

Tenga commented Mar 6, 2024

Scope

Improves an existing behavior

Compatibility

  • This is a breaking change

Feature description

Currently, when using the MSW boundaries, the callback provided to the boundary has no inferred types.

This is a slight issue when using boundaries and concurrency, as when using concurrency in Vitest, in certain cases you are required to use the local expect from the context instead of the imported expect.

Screenshot 2024-03-06 at 14 11 18

So I went digging a bit as to what the root cause of this is, and eventually I ran into this issue on the TypeScript repository. I believe this issue also describes the basic interaction between Vitest's test typings and MSW boundary typings.

I applied Daniel's suggested alternative to boundary locally

declare class SetupServerApi extends SetupServerCommonApi implements SetupServer {
    constructor(handlers: Array<RequestHandler>);
-   boundary<Fn extends (...args: Array<any>) => unknown>(callback: Fn): (...args: Parameters<Fn>) => ReturnType<Fn>;
+   boundary<Fn>(callback: Fn & Function): Fn & Function;
    close(): void;
}

and indeed the boundary callback's arguments became correctly inferred

Screenshot 2024-03-06 at 14 12 26

and I could use the expect from the context with inferred types for it.

Screenshot 2024-03-06 at 14 29 01

I'm unsure if this change would bring something undesireable or if there's something I missed. Correct me if I'm wrong, but I believe that the proposed change's outcome is an improvement as what the current types are trying to do anyway.

@Tenga Tenga added the feature label Mar 6, 2024
@kettanaito
Copy link
Member

kettanaito commented Mar 18, 2024

Hi, @Tenga. Thanks for letting me know!

I'd love to use this suggestion but it doesn't seem to work in the boundary implementation:

Screenshot 2024-03-18 at 18 35 36

T & Function is not guaranteed to be a function. I can use some help with this. @Andarist, if you have a moment, do you see something wrong with what I'm doing? The behavior is reproducible in #2100, pnpm build. Thank you.

@kettanaito
Copy link
Member

Released: v2.2.9 🎉

This has been released in v2.2.9!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

@Tenga
Copy link
Author

Tenga commented Mar 27, 2024

Thanks for looking into this, both of you!

I just got around to updating MSW to 2.2.12. Unfortunately the changes had no effect on this issue on my end.

Screenshot 2024-03-27 at 17 11 32

Parameters are still any unless manually provided to boundary.

I'm on typescript@3.4.3typescript@5.4.3. Are there any tsconfig settings needed to make this be correctly infered?

If not, can we reopen this or is the only other resolution to wait for the typescript to fix the issue linked in the first post?

@kettanaito
Copy link
Member

@Tenga, are you certain you aren't using TS 4.3? MSW doesn't support any TypeScript versions older than 4.7.

@Tenga
Copy link
Author

Tenga commented Mar 27, 2024

@Tenga, are you certain you aren't using TS 4.3? MSW doesn't support any TypeScript versions older than 4.7.

@kettanaito Sorry, yeah I mistyped that somehow. The project (and VS Code) is using typescript@5.4.3.

@Andarist
Copy link
Contributor

could you share a repro case of the problem with the current version of MSW?

@Tenga
Copy link
Author

Tenga commented Mar 27, 2024

@Andarist I've made a quick minimal repro repo that can be found here.

Below is what I get in the sample.test.ts in the project.

Screenshot 2024-03-27 at 19 08 13

The project and branch I've originally encountered this is here. Pardon the wild mess the branch is in, in case you decide to go looking. 😁

@kettanaito
Copy link
Member

kettanaito commented Mar 30, 2024

@Tenga, in the code snippet above, I'm pretty sure expect is any because the entire argument is cast to any:

-({ expect }: any)
+({ expect })

Is there any difference if you omit any explicit types?

@Andarist
Copy link
Contributor

That doesn't look like an explicit type annotation but rather like an inlay hint

@Tenga
Copy link
Author

Tenga commented Mar 30, 2024

Yup, @Andarist is correct, there is no explicit typing in the screenshot. Bright gray text is the VS Code inlay hints that I've enabled to show the inferred types clearly everywhere.

The source from the screenshot is identical to the code in the repro case I've provided in my previous post.

https://github.com/Tenga/msw-boundary-infer-repro/blob/main/sample.test.ts#L6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
3 participants