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

why does eslint require await for synchronous request and reply methods in VS code? #775

Closed
budarin opened this issue Oct 18, 2022 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@budarin
Copy link

budarin commented Oct 18, 2022

💬 Question here

here the code:
code

eslint error:
image

Your Environment

  • node version: 18
  • fastify version: 4.9.2
  • os: any
  • VS Code: 1.72.2
@budarin budarin added the help wanted Extra attention is needed label Oct 18, 2022
@Uzlopak
Copy link

Uzlopak commented Oct 18, 2022

Seems to be, that typescript thinks that it is a promise and not a sync function.

Can you provide a repo with that issue?

@budarin
Copy link
Author

budarin commented Oct 18, 2022

I'll try to make a demo repo from our project

@budarin
Copy link
Author

budarin commented Oct 18, 2022

@Uzlopak here is it

@Uzlopak
Copy link

Uzlopak commented Oct 18, 2022

LOL, that project has half of the npm registry as dependencies ;).

Ok it is because FastifyReply has a .then-method with two parameters

https://github.com/fastify/fastify/blob/68914519a3fd65ce65ba8db8962940eecf7f0fc8/types/reply.d.ts#L63

so eslint assumes it is a a Promise.

https://github.com/typescript-eslint/typescript-eslint/blob/0be356bae50156ba159c70d570b030e6d94afcb1/packages/eslint-plugin/src/rules/no-floating-promises.ts#L184

It is an issue of eslint and not of Fastify.

@budarin
Copy link
Author

budarin commented Oct 18, 2022

I wanted to keep the system in the state it is in for investigation :)

But they will rightly object to me that everything that has the 'then' method is a Promise and what should I do in this situation?

@Uzlopak
Copy link

Uzlopak commented Oct 18, 2022

You mean with "they" your colleagues?

Well sorry, but it is what it is. I dont know how we could fix it on our side to get that false positive fixed. It is an issue of eslint, as it is ducktyping it too wide.

What you can do is to add a local patch and remove the then-method in reply.d.ts

@jsumners
Copy link
Member

everything that has the 'then' method is a Promise

Not so. Such things are thennables.

As covered in fastify/fastify#4246, it's out of our control.

@budarin
Copy link
Author

budarin commented Oct 18, 2022

ok, I created an issue in eslint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants