Update type signature for SubscriptionHandler. Update associated docs.

[parkerziegler]

This PR would fix #319. As @mjadczak points out, the void type is a mistake there - handler is called in useSubscription with (s.data, data), where s.data is set initially by useImmediateState to undefined: https://github.com/FormidableLabs/urql/blob/master/src/hooks/useSubscription.ts#L36. It also fixes the signature of the mutable ref inside of useRequest, which is initialized to undefined .

I'm also not sure if we were attempting to take advantage of undefined and null being sub-types of void, so feel free to push back on this change (but it seems to make sense to me).

[andyrichardson]

Makes sense 👍

Void isn't actually a type so I think it should be avoided where possible. The only place I've found it valuable is when stating "I don't care about this value" which is almost always exclusive to function arguments and their return types.

interface Props {
  onClick: (e: MouseEvent) => void;
}

[JoviDeCroock]

Looks good to me, the issue did mention something about useRequest as well, is this in scope of this PR?

[andyrichardson]

We need to standardize the casing - I vote "Urql". Capitalized (proper noun) and not as spammy/agressive as URQL.

Not something for this pr, but worth us thinking about.

[kitten]

In all of the markdown of our docs we've now gone with "urql" (also when possible stylised as monospaced) which was discussed on Slack.

We can reopen that discussion, but frankly I think optically "urql" is fine and works well, "Urql" is OK but would now be an inconsistency given the docs, and "URQL" would be highly inconsistent and also doesn't read rather nicely (definitely shouty)

[andyrichardson]

My issue with all lowercase is that there's not much emphasis on the name which leaves it feeling a little deflated. We can highlight the name in markdown which is awesome but in other places such as a tweet, the name urql just gets lost amongst the words.

Sounds like all lowercase is the standard for now so let's go with that. Maybe there are other examples of all lowercase being used for branding? If you find any forward them to me!

[andyrichardson]

Looks like our lock file was out of sync with our package.json

[parkerziegler]

This was just for the example project, so not too concerned about it for the time being. I verified the example starts up as expected 👍

[parkerziegler]

@JoviDeCroock yep, the change for useRequest was included as well, it's at the very bottom of the diff 👍

[JoviDeCroock]

@JoviDeCroock yep, the change for useRequest was included as well, it's at the very bottom of the diff 👍

I looked over it, my bad

[parkerziegler]

Looks like Sail CI may be down?

[kitten]

@parkerziegler seems to be working now. Just a failing snapshot; not sure what happened there but the stack might be inaccurate? I've got an alternative implementation for that since we're launching devtools soon and want that to be super nice and accurate 👍

[parkerziegler]

@kitten do you want me to merge this and we'll assume that #329 will fix the build once it gets merged? Or wait until #329 is merged, rebase off of it, and then merge?

[parkerziegler]

cc/ @kitten @andyrichardson @JoviDeCroock merging this with the snapshots updated to pass the build. When #329 gets merged we can update those snapshots in tests.