types: nullish error types in callback function's parameter for `after` and `ready` method by nokazn · Pull Request #5191 · fastify/fastify

nokazn · 2023-12-07T16:12:02Z

Checklist

run npm run test and npm run benchmark
tests and/or benchmarks are included
documentation is changed or added
commit message and code follows the Developer's Certification of Origin
and the Code of conduct

I think err type definition in the first parameter of the callback function for after and ready methods are insufficient.
The one for after method can be nullish and the one for ready method is optional.

See also an small example for Fastify & TS project if needed.
Thank you.

Eomm

Why 2 different approaches?

climba03003

error should be null by default. If it doesn't, better to fix it inside avvio.
https://github.com/fastify/avvio/blob/master/boot.js#L447-L448

types/instance.d.ts

mcollina

Thanks for opening a PR! Can you please add a unit test? We use tsd for typescript.

nokazn · 2023-12-09T08:44:23Z

@Eomm

Why 2 different approaches?

The both are depends on current implementation.
The parameter type of ready method seem to pass undefined to callback function as the first argument when no errors.👀

ex. https://github.com/nokazn/fastify-ts-minimum-sample

import Fastify from 'fastify';

const fastify = Fastify({
  logger: true,
});

fastify.register((server, options, done) => {
  // If you uncomment the line below, you can see error on `after` event.
  // done(new Error('something occurred'));
  done();
});

fastify.after((error) => {
  console.error({ error }, 'error has occurred on `after` event');
});
fastify.ready((error) => {
  console.error({ error }, 'error has occurred on `ready` event');
});

fastify.get('/', async (request, reply) => {
  reply.type('application/json').code(200);
  return { hello: 'world' };
});

fastify.listen({ port: 3000 }, (err, address) => {
  if (err) throw err;
  fastify.log.debug(`Server is now listening on ${address}`);
});

↓

(I also think the both should be nullish not optional as @climba03003 said)

Co-authored-by: KaKa <23028015+climba03003@users.noreply.github.com>

nokazn · 2023-12-09T09:59:22Z

@mcollina

Thanks for opening a PR! Can you please add a unit test? We use tsd for typescript.

I've added some type tests for after & ready methods in 8d1e75a. Please check them and feel free to point out if something goes wrong. Thank you.

mcollina

lgtm

Uzlopak

LGTM

github-actions · 2024-12-14T00:29:21Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

types: nullish error types in callback function's parameter

908834c

github-actions bot added the typescript TypeScript related label Dec 7, 2023

Eomm reviewed Dec 7, 2023

View reviewed changes

climba03003 requested changes Dec 8, 2023

View reviewed changes

types/instance.d.ts Outdated Show resolved Hide resolved

mcollina requested changes Dec 8, 2023

View reviewed changes

nokazn and others added 2 commits December 9, 2023 17:59

types: update ready method's parameter type

146abe9

Co-authored-by: KaKa <23028015+climba03003@users.noreply.github.com>

types: add tests for after & ready methods

8d1e75a

nokazn requested review from climba03003 and mcollina December 9, 2023 09:59

mcollina approved these changes Dec 9, 2023

View reviewed changes

Merge branch 'main' into fix/nullish-error-types

803c4d0

climba03003 approved these changes Dec 10, 2023

View reviewed changes

Uzlopak approved these changes Dec 12, 2023

View reviewed changes

Uzlopak merged commit b3a0e67 into fastify:main Dec 12, 2023

nokazn deleted the fix/nullish-error-types branch December 13, 2023 01:58

renovate bot mentioned this pull request Dec 13, 2023

chore(integration-tests): update dependency nock to v13.4.0 gatsby-uc/plugins#441

Merged

1 task

renovate bot mentioned this pull request Jan 1, 2024

chore(deps): update dependencies-major (major) technologiestiftung/renovate-config-testing#6

Closed

1 task

github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

types: nullish error types in callback function's parameter for `after` and `ready` method#5191

types: nullish error types in callback function's parameter for `after` and `ready` method#5191
Uzlopak merged 4 commits intofastify:mainfrom
nokazn:fix/nullish-error-types

nokazn commented Dec 7, 2023

Uh oh!

Eomm left a comment

Uh oh!

climba03003 left a comment

Uh oh!

Uh oh!

mcollina left a comment

Uh oh!

nokazn commented Dec 9, 2023

Uh oh!

nokazn commented Dec 9, 2023

Uh oh!

mcollina left a comment

Uh oh!

Uzlopak left a comment

Uh oh!

github-actions bot commented Dec 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

nokazn commented Dec 7, 2023

Checklist

Uh oh!

Eomm left a comment

Choose a reason for hiding this comment

Uh oh!

climba03003 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nokazn commented Dec 9, 2023

Uh oh!

nokazn commented Dec 9, 2023

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

Uzlopak left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants