Skip to content

Conversation

@kaezone
Copy link

@kaezone kaezone commented Dec 1, 2025

This PR implements npm/rfcs issue #844, adding support for relative dates in the --before config arg (e.g. --before=24h, --before=7d, etc).

@kaezone kaezone requested a review from a team as a code owner December 1, 2025 03:17
@wraithgar
Copy link
Member

The current hint for this field is [--before <date>], this is customized via the hint attribute on any given definition. Is <date> still sufficient here?

@kaezone kaezone force-pushed the feature/relative-before branch from b4449e2 to 9b1a390 Compare December 1, 2025 20:15
@kaezone
Copy link
Author

kaezone commented Dec 1, 2025

Couldn't really think of a better hint attribute that was more descriptive without being too wordy - <date/relative> maybe? I think date is still fairly applicable as long as the rest of the doc string notes using relative dates.

@kaezone kaezone requested a review from wraithgar December 1, 2025 20:52
@wraithgar
Copy link
Member

I think date is still fairly applicable as long as the rest of the doc string notes using relative dates.

That seems fine to me, just wanted to bring it up in case I was missing something obvious.

@kaezone
Copy link
Author

kaezone commented Dec 1, 2025

Just curious, would this be able to fit in the 11.7.0 release (if accepted of course) or is it a bit late?

@wraithgar
Copy link
Member

The reason this hasn't been merged yet is because of hesitation with the "loosely defined human parsable date range" part of the parameter. Historically things like this feel helpful but are not. As long as folks have a consistent way to set a minimum age I think being more rigorous in what this value can be is in order.

Defining this as an integer value of hours may be sufficient. I think it's worth waiting on and considering input from others with UX knowledge.

@wraithgar
Copy link
Member

Prior art here is:

  • pnpm, which defines this as an integer of minutes
  • dependabot, which explicitly names the parameter as "days" and accepts integer values

@kaezone
Copy link
Author

kaezone commented Dec 3, 2025

Defining this as an integer value of hours may be sufficient.

Yeah I agree that'd probably be more consistent (and easier to explain in docs) - would matching pnpm with an integer of minutes be ideal?

Will wait for input before doing any more tweaking on this.

@wraithgar
Copy link
Member

Will wait for input before doing any more tweaking on this.

Best option, thanks!

@karlhorky
Copy link
Contributor

karlhorky commented Dec 4, 2025

As other users mentioned in the alternative PR #8825, this approach of extending --before misses the feature of excluding packages, patterns and specific versions of packages, which is possible with pnpm's minimumReleaseAgeExclude:

Maybe it makes sense to reconsider this alternative PR #8825, as it follows the standard patterns introduced by pnpm minimumReleaseAge and Bun minimumReleaseAge (although original minimumReleaseAge naming credit goes to Renovate).

@kaezone
Copy link
Author

kaezone commented Dec 4, 2025

FWIW I'm not particularly attached to my PR, we're just looking for any variation of this feature at my workplace.

@wraithgar
Copy link
Member

Thanks @kaezone it is still a good PR and we're glad you made it. This is a topic that's obviously top of mind for a lot of folks and this PR has become a great focal point for the discussion.

@AsdarhTheBest
Copy link

As other users mentioned in the alternative PR #8825, this approach of extending --before misses the feature of excluding packages, patterns and specific versions of packages, which is possible with pnpm's minimumReleaseAgeExclude:

Maybe it makes sense to reconsider this alternative PR #8825, as it follows the standard patterns introduced by pnpm and Bun (although original minimumReleaseAge naming credit goes to Renovate).

Yuuuup, I made a Github account only for joining this discussion cuz was interesting, I was searching for if npm has a minimum age release rule or something and found this.

I comply with @karlhorky . @wraithgar you should reconsider re-opening #8825 PR. Implements a minimum release age policy very similar as pnpm's and Bun's.

Not pretending to ofend, this PR is really good, but I don't think that is what the community wants to see on NPM. After all, the final usser/developer is the one who wants to code without caring about vulnerabilities in dependencies/sub-dependencies and set a minimum required age for the releases of the packages and with that, they can be "safer" in some way.

As many people say in #8825, the feature implemented at this PR, has no form of excluding packages from this policy.

In my point of view as a developer, I would prefer having the possibility of excluding some packages.
Example:

  • Express: Minimum release age
  • React: Maybe, I dont want to include this policy, so exclude the package.

I believe both implementations should be implemented, but for a more complete implementation, in my opinion and that of several users and even contributors, the best option currently would be to implement PR #8825. However, I also don't want this PR to be closed, as it can also be improved beforehand, allowing both implementations to work harmoniously together.

@kaezone
Copy link
Author

kaezone commented Dec 5, 2025

I'm generally in agreement here as well - the other PR looks quite a bit more robust.

This PR was intended as the minimum necessary to extend the current functionality to help reduce a risk I'm working on (and presumably one many other dev shops face), but the other one provides quite a bit more in terms of QOL.

For what it's worth, I primarily work in the .NET appsec and GRC spaces, so I'd tend to lean towards the opinion of those who spend more time in the JS/TS ecosystem :)

@wraithgar
Copy link
Member

Naming of the parameter aside, the approach in #8825 is not correct. This is a parameter that is parsed in npm-pick-manifest. The cli itself just gets the intent from the user and passes it down. npm-pick-manifest is where the "get me a package based on this spec" logic lives, and this before param then limits the time frame of available versions it can "see". Arborist, pacote, and the cli don't care about any of that.

This is one of the reasons extending before makes sense. It already does what we want.

I realize we are getting bogged down a bit in the naming of the parameter, and not the implementation. I don't think anyone involved doesn't want this, it's just a matter of having it work where it is supposed to. There is also the matter of the exclusion option, which is a bit out of scope for this initial extension of the feature.

@tounsils
Copy link

New to the repo here — really appreciate the thoughtful discussion on this PR. Extending --before at the npm-pick-manifest level makes sense architecturally, and it’s helpful to see the tradeoffs discussed so clearly. I agree that starting with a well-defined, consistent minimum-age mechanism and iterating later (e.g. exclusions) feels like a pragmatic path forward. Thanks to everyone involved for the transparency and context.

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

Successfully merging this pull request may close these issues.

5 participants