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

Suggestion: Export withPromise for easier typing in typescript tests #3133

Open
probablykabari opened this issue Apr 5, 2023 · 2 comments
Open
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release

Comments

@probablykabari
Copy link

https://github.com/urql-graphql/urql/blob/9cdb74b03e07d46e056ef023d1543f24a823ec55/packages/core/src/utils/streamUtils.ts

If you're coming from v3 -> v4 in typescript the following results in a type error because this is a Source<OperationResult> instead of OperationResultSource<OperationResult>:

   // on a mock Urql client
    executeQuery: () => {
       const result = makeResult(....) // OperationResult
        return fromValue(result) // Source<OperationResult>
    };

It's a small thing but I think it is better to have access to the util than use as OperationResultSource<OperationResult> for anyone consuming the library.

@probablykabari probablykabari added the future 🔮 An enhancement or feature proposal that will be addressed after the next release label Apr 5, 2023
@kitten
Copy link
Member

kitten commented Apr 5, 2023

Personally, I'm a little torn on this.

Of course, turning a Source<T> into an OperationResultSource<T> is “trivial” and withPromise could be exported, there's something to be said about:

Rather, it can actually, I believe, lead to uncaught bugs in tests. The reason being that it encourages testing a single case one by one.

So, I think as OperationResultSource<T> is preferable for the time being because it enforces the "conscious" decision to mock the Client.

However, this kind of mocking isn't perfect and doesn't handle:

  • stale behaviour while replaying results
  • deduplication and concurrent results
  • loading behaviour, unless a delay is added (which is too complex today, imho)

In short, I'd much rather bite the bullet and think about mocking utilities that emulate the full Client more closely. The reason being that the Client has become more burdened with control-flow / event hub logic, and that a mock with a cast is imho 100% fine for the purpose of mocking the Client for our bindings.

So, instead, I think it's time to maybe think about @urql/testing and to add some test utilities, which allow for:

  • an exchange that can play a single result, either immediately, on a one-tick delay, or imperatively
  • an exchange that can delay results via an external test scheduler (i.e. grouped with React's act or Vue's equivalent)
  • an exchange that can replay different results depending on inputs (to replace @urql/storybook-addon's approach)
  • and maybe some more Client utilities to extend it into MockClient potentially (?)

How does that sound?
I think this gets us further than exposing withPromise, which I think would just be a crutch

@probablykabari
Copy link
Author

probablykabari commented Apr 6, 2023

I spoke too soon, I'm in the midst of upgrading and withPromise definitely doesn't work with subscriptions (as you implied).

+additional side note, using as OperationResultSource<T> makes it impossible to test anything that uses the client directly.

I agree with the testing library. Having used Urql since 1.x I have bits and pieces of test utilities all over the place in different projects and having a more first-class library would be a great addition. I like the idea of doing it via an exchange, we do something similar to test relay pagination. Happy to take a stab at making an initial PR for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
future 🔮 An enhancement or feature proposal that will be addressed after the next release
Projects
None yet
Development

No branches or pull requests

2 participants