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

Add policy on using caret (^) or tilde (~) with dependencies #290

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

UlisesGascon
Copy link
Member

@UlisesGascon UlisesGascon commented Oct 22, 2024

Policy on using caret (^) or tilde (~) with dependencies

  • Establishes policy for using caret (^) or tilde (~) for dependencies that we maintain while avoiding its use for third-party dependencies.
  • Explains the differences between caret (^) and tilde (~) version ranges.
  • Documents the rationale, alternatives considered, and implementation plan.

Context

Signed-off-by: Ulises Gascon <ulisesgascongonzalez@gmail.com>
- [expressjs/express#6017 (comment)](https://github.com/expressjs/express/issues/6017)

## Decision
We will adopt a policy where the caret (`^`) symbol is used for dependencies owned within the Express ecosystem (e.g., `body-parser` for Express), but continue to avoid using it for third-party dependencies that we do not maintain.
Copy link
Contributor

Choose a reason for hiding this comment

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

why not also for third party deps? most maintainers don't backport fixes to previous minors.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, we should always use ^ and use dependencies/authors we trust, no need to fight the ecosystem.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed!

Suggested change
We will adopt a policy where the caret (`^`) symbol is used for dependencies owned within the Express ecosystem (e.g., `body-parser` for Express), but continue to avoid using it for third-party dependencies that we do not maintain.
We will adopt a policy where the caret (`^`) symbol is used for dependencies all dependencies.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
We will adopt a policy where the caret (`^`) symbol is used for dependencies owned within the Express ecosystem (e.g., `body-parser` for Express), but continue to avoid using it for third-party dependencies that we do not maintain.
We will adopt a policy where the caret (`^`) symbol is used for all dependencies.


**Alternatives Considered:**
- **Alternative 1:** Use `^` for all dependencies, including third-party libraries.
- **Reason for rejection:** This could increase the risk of unintended regressions and security issues from third-party updates.
Copy link
Contributor

Choose a reason for hiding this comment

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

imo this risk is lower than the risk of dependency vulnerability/regression fixes from not being available to downstream users.

Copy link
Member

@blakeembrey blakeembrey left a comment

Choose a reason for hiding this comment

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

TL;DR I think we should try to be high trust and good examples of OSS by treating all dependencies the same.

- [expressjs/express#6017 (comment)](https://github.com/expressjs/express/issues/6017)

## Decision
We will adopt a policy where the caret (`^`) symbol is used for dependencies owned within the Express ecosystem (e.g., `body-parser` for Express), but continue to avoid using it for third-party dependencies that we do not maintain.
Copy link
Member

Choose a reason for hiding this comment

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

Agreed, we should always use ^ and use dependencies/authors we trust, no need to fight the ecosystem.

- For our own dependencies that adhere strictly to semver, `^` is preferred over `~`, while for third-party libraries, a stricter versioning strategy is maintained to prevent unexpected regressions.

**What will not be done?**
- We will not use `^` or `~` for external dependencies that are not maintained by the Express organization, as they could introduce unexpected changes.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure there's any reason to use ~ either, I think we would have used it instead of ^ due to supported NPM versions.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's indeed the only reason express used ~, because v4 came out at a time when not every npm version supported ^.

Copy link
Member

@wesleytodd wesleytodd left a comment

Choose a reason for hiding this comment

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

If @UlisesGascon is alright with the suggestions to use ^ for all, I tried to make change suggestions to everywhere I noticed where it was referenced. Personally I think the conservative approach used in earlier days is not necessary anymore and so am in favor of using it for all deps.

- Express TC (@expressjs/express-tc)

## Context
Historically, the Express project has avoided using the caret (`^`) in the `package.json` files for its own dependencies. This decision aims to review whether this practice should continue or if adjustments are needed.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Historically, the Express project has avoided using the caret (`^`) in the `package.json` files for its own dependencies. This decision aims to review whether this practice should continue or if adjustments are needed.
Historically, the Express project has avoided using the caret (`^`) in the `package.json` files for its own dependencies. This proposal aims to review whether this practice should continue or if adjustments are needed.

Clarifying the policy on using caret (`^`) helps to ensure consistency across the Express ecosystem, reduce unnecessary maintenance, and prevent unexpected dependency updates. It also addresses concerns about the balance between keeping dependencies up-to-date and avoiding regressions.

**What problem does it solve or avoid?**
This decision aims to avoid unexpected updates and regressions from external dependencies while reducing the maintenance burden of frequently updating pinned dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This decision aims to avoid unexpected updates and regressions from external dependencies while reducing the maintenance burden of frequently updating pinned dependencies.
This decision aims to reduce the maintenance burden of frequently updating pinned dependencies.

We will adopt a policy where the caret (`^`) symbol is used for dependencies owned within the Express ecosystem (e.g., `body-parser` for Express), but continue to avoid using it for third-party dependencies that we do not maintain.

**What will be done?**
- Update the `package.json` files to use `^` for our own dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Update the `package.json` files to use `^` for our own dependencies.
- Update the `package.json` files to use `^` for all dependencies (both prod and dev).


**What will be done?**
- Update the `package.json` files to use `^` for our own dependencies.
- Continue using fixed versions (no caret or tilde) for external dependencies to minimize risks of unintended updates.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Continue using fixed versions (no caret or tilde) for external dependencies to minimize risks of unintended updates.


## Implementation

- **Phase 1**: Update `package.json` files across all packages to apply `^` for our own dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- **Phase 1**: Update `package.json` files across all packages to apply `^` for our own dependencies.
- **Phase 1**: Update `package.json` files across all packages to apply `^` for all dependencies.

@UlisesGascon
Copy link
Member Author

UlisesGascon commented Oct 26, 2024

Worth mentioning that the caret (^) is not supported on Node@0.8, as noted in this comment. Therefore, this change might be considered semver-major in many scenarios unless we decide that dropping support for Node@0.8 is not a major change. 😄

@ljharb
Copy link
Contributor

ljharb commented Oct 27, 2024

@UlisesGascon it's not supported on the npm that comes with node 0.8, but if you use nvm install-latest-npm to get the latest one, then it works just fine :-)

either way, i think it can be fine if you need a newer node to install it as long as it works in old node (albeit, not ideal)

@bjohansebas
Copy link
Member

So we need to prepare the CI for the packages that still support node.js@0.8 before making any changes to the dependencies

@ljharb
Copy link
Contributor

ljharb commented Oct 27, 2024

Yes, for any package that isn't already declaring engines, the first step should be testing (manual or automated) to determine on which versions it's already working - because those are the engines that it's non-breaking to declare.

@bjohansebas
Copy link
Member

@expressjs/express-tc, the way forward is to update the CI, or do we prefer to do this in a major version when support for Node.js 0.8 is removed?

@jonchurch jonchurch added the ADR label Nov 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Discussion: Using caret (^) with our own dependencies
6 participants