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

Request Cookies Should Be Typed As string | undefined #189

Closed
2 tasks done
ryanagillie opened this issue Jul 1, 2022 · 3 comments
Closed
2 tasks done

Request Cookies Should Be Typed As string | undefined #189

ryanagillie opened this issue Jul 1, 2022 · 3 comments
Labels
good first issue Good for newcomers

Comments

@ryanagillie
Copy link
Contributor

ryanagillie commented Jul 1, 2022

Prerequisites

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

Fastify version

3.29.0

Plugin version

6.0.0

Node.js version

16.15.1

Operating system

Windows

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

Windows 11 Pro Build 22000.739

Description

Right now when using the package any cookie is always typed as a string when it could be undefined.

Pretty simple fix, just changing the type from cookies: { [cookieName: string]: string }; to cookies: { [cookieName: string]: string | undefined };

(if you wanted to make it more TypeScript-y you could do cookies: Record<string, string | undefined>;

Steps to Reproduce

fastify.get('/', async (reqeust, reply) => {
  const { someCookieThatDoesNotExist } = request.cookies; // someCookieThatDoesNotExist: string
  console.log(someCookieThatDoesNotExist); // undefined
  console.log(typeof someCookieThatDoesNotExist); // undefined
  
  return { hello: 'world' }
});

Expected Behavior

No response

@ryanagillie ryanagillie changed the title Request Cookies Should Be Typed as string | undefined Request Cookies Should Be Typed As string | undefined Jul 1, 2022
@mcollina
Copy link
Member

mcollina commented Jul 1, 2022

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mcollina mcollina added the good first issue Good for newcomers label Jul 1, 2022
@ryanagillie
Copy link
Contributor Author

Absolutely! I'll get right on it :D

@ryanagillie
Copy link
Contributor Author

#190 addresses this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants