maticzav / nookies

🍪 A set of cookie helpers for Next.js
2.28k stars 76 forks source link

`express` and `cookie` typings are not available automatically #492

Open O4epegb opened 3 years ago

O4epegb commented 3 years ago

nookies functions depends on typings from other libraries, for example:

import * as cookie from 'cookie';
import * as express from 'express';
import * as next from 'next';

export declare function setCookie(ctx: Pick<next.NextPageContext, 'res'> | {
    res: next.NextApiResponse;
} | {
    res: express.Response;
} | null | undefined, name: string, value: string, options?: cookie.CookieSerializeOptions): {};

But it seems like they are not installed automatically because they are in devDependencies:

  "devDependencies": {
    "@types/cookie": "0.4.1",
    "@types/express": "4.17.13",
    // ...
  },

Maybe they should be moved to the regular dependencies instead? What is the best practice here?

mecaota commented 3 years ago

I also encountered the same symptom. In my case, the type inference in the setCookie option did not work, so I solved the problem by adding manually to the project I was using.

yarn add -D @types/cookie @types/express

~However, I think it can be fixed by adding include node_modules to tsconfig at nookies.~

https://github.com/maticzav/nookies/blob/383cf07f3b480bb46b6cc2e6b54b48a711309f2a/packages/nookies/tsconfig.json#L7

"include": ["src", "node_modules"] 
O4epegb commented 3 years ago

How would that help?

I don't think it will because this packages are not even installed when you install nookies, so it does not matter what you include, you would still need to install them manually.

In my understanding this is not Typescript related, more like NPM related thing

mecaota commented 3 years ago

I don't think it will because this packages are not even installed when you install nookies, so it does not matter what you include, you would still need to install them manually.

In my understanding this is not Typescript related, more like NPM related thing

You were right. I was wrong.

So, I think the solution is to move the some definition package in devDependencies. If so, I'm thinking of issuing a PR.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

O4epegb commented 2 years ago

I think it's still actual