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

Fix missing implicit types #211

Merged
merged 3 commits into from
Oct 6, 2023
Merged

Fix missing implicit types #211

merged 3 commits into from
Oct 6, 2023

Conversation

mckelveygreg
Copy link
Contributor

@mckelveygreg mckelveygreg commented Oct 5, 2023

We have really appreciated this lib, and have been using it since it came out! I think I found a typing fix to make it even more usable.

When trying to bundle up the generated code with tsc --declaration=true, ts couldn't figure out the disableQuery type and the BaseInfiniteQueryOptions type. This is able to be resolved if the exported queries are explicitly typed.

This also will resolve #209 🎉

Let me know if there is anything else I can do to this PR!

When trying to bundle up the generated code with `tsc --declaration=true`, ts couldn't figure out the disableQuery type and the BaseInfiniteQueryOptions. This is able to be resolved if the exported querys are explicitly typed
@CLAassistant
Copy link

CLAassistant commented Oct 5, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@timostamm timostamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Greg, I believe this is the correct fix, thanks for the contribution!

We did the same in protobuf-es in bufbuild/protobuf-es#398.

I'll defer to @paul-sachs regarding any implementation details.

@timostamm timostamm changed the title fix: fix missing implicit types Fix missing implicit types Oct 6, 2023
@@ -74,13 +75,22 @@ const generateServiceFile =
);
f.print();


const unaryFunctionType = (method: DescMethod) => [f.import('UnaryFunctions', '@connectrpc/connect-query'), `<${method.input.name}, ${method.output.name}>`]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mckelveygreg This looks good, though I think we can simplify by using the helper type UnaryFunctionsWithHooks instead of UnaryFunctions<...> & UnaryHooks<...>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅 Ha, yeah that is much more straight forward!
Thanks!

@paul-sachs paul-sachs merged commit 6065baa into connectrpc:main Oct 6, 2023
5 checks passed
@mckelveygreg mckelveygreg deleted the fix/unfound-implicit-types branch October 6, 2023 17:21
@paul-sachs paul-sachs mentioned this pull request Oct 6, 2023
paul-sachs added a commit that referenced this pull request Oct 6, 2023
## What's Changed
* Remove experimental plugin by @paul-sachs in
#178
* Add ESM wrapper to avoid dual package hazard by @timostamm in
#180
* Fix missing implicit types by @mckelveygreg in
#211

## New Contributors
* @mckelveygreg made their first contribution in
#211

**Full Changelog**:
v0.5.1...v0.5.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

disableQuery unique symbol stopping typescript declaration
4 participants