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

chore: move mongodb to devDeps and add it to peerDeps #226

Closed
wants to merge 1 commit into from

Conversation

rtritto
Copy link

@rtritto rtritto commented Oct 22, 2023

Checklist

Copy link
Member

@gurgunday gurgunday left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

If I understand peerDeps correctly, this risks not installing mongoDB if a user installs packages using npm ci --omit=dev

@rtritto
Copy link
Author

rtritto commented Oct 22, 2023

The default value of omit option is dev.
From npm version >= 7 (Node version >= 16) peer dependencies are autoinstalled: source.
Imply that mongodb, as peer dependency, will be installed with omit=dev.

@mcollina
Copy link
Member

Unfortunately the minimum supported version of this module is Node v14.

@mcollina mcollina added the semver-major Issue or PR that should land as semver major label Oct 23, 2023
@mcollina
Copy link
Member

Generically we prefer to stay away from peerdependencies because they create more problems than what they are worth.

@rtritto
Copy link
Author

rtritto commented Oct 23, 2023

Unfortunately the minimum supported version of this module is Node v14.

For mongodb v6 the minimum supported Node.js version is v16.20.1 (source).

@gurgunday
Copy link
Member

Oof, that's correct, good catch

A PR that downgrades it is welcome

@mcollina
Copy link
Member

I would really prefer avoiding peerDependencies anyway. They are just a source of headaches for maintainers.

@rtritto
Copy link
Author

rtritto commented Oct 23, 2023

Node v14 isn't mantained, when it will be removed?

@gurgunday
Copy link
Member

When Fastify v5 is released (probably by the end of this year)

@simoneb simoneb mentioned this pull request Mar 20, 2024
4 tasks
@climba03003 climba03003 added the wontfix This will not be worked on, the behavior is intended label Apr 1, 2024
@climba03003 climba03003 closed this Apr 1, 2024
@climba03003
Copy link
Member

I am closing the PR because we avoid using peerDependencies in any case.
So, it is not going to merge in anytime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major Issue or PR that should land as semver major wontfix This will not be worked on, the behavior is intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants