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

@types/pg as dev dependency #218

Closed
thernstig opened this issue Sep 29, 2021 · 7 comments
Closed

@types/pg as dev dependency #218

thernstig opened this issue Sep 29, 2021 · 7 comments

Comments

@thernstig
Copy link

Currently in https://github.com/voxpelli/node-connect-pg-simple/blob/main/package.json it has these dependencies:

  "dependencies": {
    "@types/pg": "^8.6.1",
    "pg": "^8.7.1"
  },

@types/pg should rather be a devDependency, as it makes no sense to include that in production code.

@kylewillmon
Copy link

I came here to report the same issue, but decided to check first for a possible explanation.

Why was this issue closed as completed? The @types/pg dependency is still present...

@thernstig
Copy link
Author

@kylewillmon I cannot remember why I decided to do it. Possibly due to that having it as a production dependency means it gets shipped as part of the npm install done by projects, so they can reference types directly e.g. via JSDoc comments as TypeScript or similar. Without having to install @types/pg separately. But I assume having it separate is still wanted.

An even better solution imo would be to bundle the declaration files in this repo and ship them with the installation, as the DefinitelyTyped (@types) repo is often considered a "backup" in case projects do not have their own types. Having types in the actual main project is preferable and recommended afaik.

🤷

@kylewillmon
Copy link

In that case, I will open a PR. Thanks

@voxpelli
Copy link
Owner

voxpelli commented Nov 1, 2023

The type situations around express session has been a bit messy. I think I added it as a dependency as I normally try to publish types for my modules, but for this module I haven't yet gotten to that

@voxpelli voxpelli reopened this Nov 1, 2023
@voxpelli
Copy link
Owner

voxpelli commented Nov 1, 2023

It was moved there in 7d20257, but I can't find a justification for doing so 🤔

@voxpelli
Copy link
Owner

voxpelli commented Nov 1, 2023

Fixed in ea4a9c1

@voxpelli voxpelli closed this as completed Nov 1, 2023
@voxpelli
Copy link
Owner

voxpelli commented Nov 1, 2023

Released as 9.0.1

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

No branches or pull requests

3 participants