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 inferring the correct fn type #218

Merged
merged 6 commits into from
Jul 18, 2023
Merged

fix inferring the correct fn type #218

merged 6 commits into from
Jul 18, 2023

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Jul 16, 2023

Fixes #217

I got help from @webstrand. Props to him.

It should now work.

According to webstrand

Typescript isn't currently capable of partial inference
if you specify any of the type parameters, the rest get filled in with their defaults
this is why you see some typescript-specific apis doing stuff like
foo()("blah") because the first function call doesn't perform any inference, but typescript lets the second function call infer from its parameters

Maybe needs a new paragraph regarding typing in the readme.md were we basically write, that they should not use the generics but specify the types on the parameters, when calling fastifyPlugin?

Checklist

@Uzlopak Uzlopak linked an issue Jul 16, 2023 that may be closed by this pull request
2 tasks
@Uzlopak

This comment was marked as outdated.

@Uzlopak Uzlopak marked this pull request as draft July 18, 2023 01:38
@Uzlopak Uzlopak marked this pull request as ready for review July 18, 2023 03:41
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Jul 18, 2023

@fastify/typescript

Can you have a look please?

I am actually highly confident, that this is the solution.

@fox1t
Copy link
Member

fox1t commented Jul 18, 2023

Hey. This approach is new to me: instead of using polymorphism, we are going with a generic inference. I like it! I am wondering if there are other points in the Fastify ecosystem where we can use the same approach (the same one we did with the infamous triplet and the export = thing when we discovered it)

@mcollina mcollina merged commit 17f1593 into master Jul 18, 2023
15 checks passed
@Uzlopak Uzlopak deleted the fix-217 branch July 18, 2023 15:51
@mcollina
Copy link
Member

mcollina commented Aug 2, 2023

How? Could you open a fresh issue?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Aug 2, 2023

@mcollina Probably mixed use of async and callback. like we had in #220
Should we add the infomation, that you should not mix both approaches?

@mcollina
Copy link
Member

mcollina commented Aug 2, 2023

Should we add the infomation, that you should not mix both approaches?

yes

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Aug 2, 2023

created an issue #221

renovate bot referenced this pull request in specfy/specfy Aug 4, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [fastify-plugin](https://togithub.com/fastify/fastify-plugin) |
[`4.5.0` ->
`4.5.1`](https://renovatebot.com/diffs/npm/fastify-plugin/4.5.0/4.5.1) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/fastify-plugin/4.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/fastify-plugin/4.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/fastify-plugin/4.5.0/4.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/fastify-plugin/4.5.0/4.5.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>fastify/fastify-plugin (fastify-plugin)</summary>

###
[`v4.5.1`](https://togithub.com/fastify/fastify-plugin/releases/tag/v4.5.1)

[Compare
Source](https://togithub.com/fastify/fastify-plugin/compare/v4.5.0...v4.5.1)

#### What's Changed

- docs: rename next callback to done to make consistent with core by
[@&#8203;58bits](https://togithub.com/58bits) in
[https://github.com/fastify/fastify-plugin/pull/210](https://togithub.com/fastify/fastify-plugin/pull/210)
- chore(.gitignore): add bun lockfile by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-plugin/pull/211](https://togithub.com/fastify/fastify-plugin/pull/211)
- build(deps-dev): bump tsd from 0.25.0 to 0.27.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-plugin/pull/212](https://togithub.com/fastify/fastify-plugin/pull/212)
- build(deps-dev): bump tsd from 0.27.0 to 0.28.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-plugin/pull/213](https://togithub.com/fastify/fastify-plugin/pull/213)
- build(deps-dev): bump
[@&#8203;fastify/type-provider-typebox](https://togithub.com/fastify/type-provider-typebox)
from 2.4.0 to 3.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-plugin/pull/214](https://togithub.com/fastify/fastify-plugin/pull/214)
- ci: only trigger on pushes to main branches by
[@&#8203;Fdawgs](https://togithub.com/Fdawgs) in
[https://github.com/fastify/fastify-plugin/pull/215](https://togithub.com/fastify/fastify-plugin/pull/215)
- build(deps-dev): bump
[@&#8203;types/node](https://togithub.com/types/node) from 18.16.5 to
20.1.0 by [@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/fastify/fastify-plugin/pull/216](https://togithub.com/fastify/fastify-plugin/pull/216)
- fix inferring the correct fn type by
[@&#8203;Uzlopak](https://togithub.com/Uzlopak) in
[https://github.com/fastify/fastify-plugin/pull/218](https://togithub.com/fastify/fastify-plugin/pull/218)

#### New Contributors

- [@&#8203;58bits](https://togithub.com/58bits) made their first
contribution in
[https://github.com/fastify/fastify-plugin/pull/210](https://togithub.com/fastify/fastify-plugin/pull/210)

**Full Changelog**:
fastify/fastify-plugin@v4.5.0...v4.5.1

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 4pm on friday,before 9am on
monday,every weekend" in timezone Europe/Paris, Automerge - At any time
(no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/specfy/specfy).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMS4wIiwidXBkYXRlZEluVmVyIjoiMzYuMTEuMCIsInRhcmdldEJyYW5jaCI6ImNob3JlL3Jlbm92YXRlQmFzZUJyYW5jaCJ9-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

Broken Typescript types when defining options and done
4 participants