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

Update API documentation on CombinedError. #106

Closed
parkerziegler opened this issue Sep 27, 2019 · 2 comments · Fixed by #109
Closed

Update API documentation on CombinedError. #106

parkerziegler opened this issue Sep 27, 2019 · 2 comments · Fixed by #109
Labels
good first issue Good for newcomers

Comments

@parkerziegler
Copy link
Contributor

Currently our API documentation on CombinedError is non-existent. This is an important part of the urql API and deserves to be covered in more depth. API documentation should include the following:

  • Type signature and description of the CombinedError.combinedError record.
  • Examples showing how to pattern match combinedError on the networkError and graphQLErrors fields.
  • Example showing the message field added in Expose message field on combinedError #105.

In addition, we should add a .rei file for this module as long as we're here.

@parkerziegler parkerziegler added the good first issue Good for newcomers label Sep 27, 2019
@Schmavery
Copy link
Collaborator

Schmavery commented Sep 27, 2019

One thing that I found a little funny with this module is that the "type t" is the js version of this type, which afaict isn't really used that much by users. Mostly I would interact with the combinedError type (the record version). Caused some confusion once when trying to annotate something with UrqlCombinedError.t fwiw
Not a big deal but unless I'm missing something, could be worth swapping the name of the user-facing type to be t.

@parkerziegler
Copy link
Contributor Author

parkerziegler commented Sep 27, 2019

Definitely agreed, that was an oversight on my part @Schmavery. I think initially I was planning to leave the Js.t as the exposed API, but now that the record is what people are actually working with I'm going to adjust the API such that what today is UrqlCombinedError.combinedError becomes UrqlCombinedError.t.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
2 participants