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

Bump to 4.24.1 breaks build #5094

Closed
2 tasks done
rebelchris opened this issue Oct 14, 2023 · 9 comments · Fixed by #5096
Closed
2 tasks done

Bump to 4.24.1 breaks build #5094

rebelchris opened this issue Oct 14, 2023 · 9 comments · Fixed by #5096

Comments

@rebelchris
Copy link

rebelchris commented Oct 14, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.24.1

Plugin version

No response

Node.js version

18.16

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

14.0

Description

If I bump from 4.23.2 to 4.24.1 I get following build error.
Not 100% sure if it's related, but seems to be the stuff introduced here.

Error I get:

node_modules/fastify/types/instance.d.ts:146:3 - error TS1169: A computed property name in an interface must refer to an expression whose type is a literal type or a 'unique symbol' type.

146   [Symbol.asyncDispose](): Promise<undefined>;
      ~~~~~~~~~~~~~~~~~~~~~

node_modules/fastify/types/instance.d.ts:146:11 - error TS2339: Property 'asyncDispose' does not exist on type 'SymbolConstructor'.

146   [Symbol.asyncDispose](): Promise<undefined>;
              ~~~~~~~~~~~~

Found 2 errors in the same file, starting at: node_modules/fastify/types/instance.d.ts:146

Wondering if anyone can shed some light on it, or advise.

Steps to Reproduce

I don't have bare minimal reproduction, but you can see the fail from bump here.

Expected Behavior

No response

@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 14, 2023

asyncDispose was introduced with #5082 by @arthurfiorette

If I remember correctly Symbol.asyncDispose was added to the node typings last week or so, by uncommenting the already existing comments. Dont know if it is released already.

Maybe by using latest nodejs v20 typings it will not throw typescript errors. Dont know if asyncDispose is available for node v18, or if the typings were backported to v18.

We could try to fix the typings by adding a ts-ignore above asyncDispose.

But I wonder... What if e.g. node 14 does not have asyncDispose. Does this mean that fastify instances have a potential undefined attribute, when Symbol.asyncDispose is not found?

@Uzlopak
Copy link
Contributor

Uzlopak commented Oct 14, 2023

Oh wait. Merged 3 hours ago

nodejs/node#50123

It needs typescript 5.2?

@arthurfiorette
Copy link
Contributor

It probably only breaks with skipLibCheck: false. We could re-ship it with a //@ts-ignore comment or we could extend the global Symbol namespace ourselves.

@arthurfiorette
Copy link
Contributor

@rebelchris are you using nodejs 18, right? This error only appears if @types/node package is installed with a version below 17. Try updating your @types/node version to 18+ or diasble skipLibCheck inside your tsconfig.

@rebelchris
Copy link
Author

@arthurfiorette my nodes/rypes are set to 18.16.17

@arthurfiorette
Copy link
Contributor

And what typescript version?

@rebelchris
Copy link
Author

Typescript 5.2.2

@mcollina
Copy link
Member

released v4.24.2

@rebelchris
Copy link
Author

Thanks everyone for the swift actions.
Just wanted to inform you on 4.24.2 it does work 🙏

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 a pull request may close this issue.

4 participants