Added graphql hoc missing data type generic

[EricMcRay]

Fix for #1920

Checklist:

• If this PR is a new feature, please reference an issue where a consensus about the design was reached (not necessary for small changes)
• Make sure all of the significant new logic is covered by tests
• If this was a change that affects the external API used in GitHunt-React, update GitHunt-React and post a link to the PR in the discussion.

/label typescript

[apollo-cla]

@EricMcRay: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/

[rosskevin]

Order seems to be always <TData, TVariables elsewhere right? Any reason we should change the order on this? It could be type-breaking with inserting TData first but perhaps that is warranted for consistency's sake?

[EricMcRay]

@rosskevin I aware of the order. The reason, I put TData as last parameter with any, I dont want to do any breaking change in types like you said and get the fixed types in next minor version. If you planned to merge this to major version, you can change the order to preserve consistency.

[danilobuerger]

I don't think thats a great idea. It will cause more pain down the road if suddenly TData and TVariables are switched for that one case. This will cause many developer hours wasted on bug hunting as people expect the proper order. In my opinion, it should rather be a breaking change or export new types and deprecate the old ones.

[rosskevin]

This looks like it is used internally as far as I can tell from a search of the codebase. I would be ok with directly using <TData, TVariables, but if there are objections we can do something different.

[danilobuerger]

But it was exported, so people could be using it. Maybe just rename them to be on the safe side? Then people who use them would catch the change immediately.

[rosskevin]

I think with the rename and ordering change, this will be good to merge.

[danilobuerger]

@EricMcRay do you intend on finishing this to get it merged?

[EricMcRay]

I updated typings as you talked. I extended GraphqlQueryControls from QueryControls to prevent any inconsistency in the future.

[danilobuerger]

Please change all usages of GraphqlQueryControls to QueryControls.

[EricMcRay]

I changed all usages and added TData generic for better typing but I am not sure it will break something. I don't have good knowledge about all code base. Can you check and confirm changes?

[danilobuerger]

Great stuff @EricMcRay . Thanks! Just some minor changes and its ready to go.

[hwillson]

It looks like we're all set here - @danilobuerger @rosskevin would you agree?