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

[RRFC] --save-peer flag #95

Closed
ruyadorno opened this issue Jan 28, 2020 · 7 comments
Closed

[RRFC] --save-peer flag #95

ruyadorno opened this issue Jan 28, 2020 · 7 comments

Comments

@ruyadorno
Copy link
Contributor

This was brought up by @ljharb during one of our conversations and I'm putting up this RRFC in order to collect early comments and see if anyone can expand upon it and submit a proper RFC in the future.

Essentially the idea here is that it would be nice for the cli to have a more robust or advanced support to saving peer deps ranges from the cli rather than manually handling the package.json file.

npm install react@16.2 --save-peer
# would yield a "react": "16.0 || 16.2" in package.json as result
# in a diff package.json containing react as a dep with a range: `~16.0 || ~16.1`
npm install react@~16.5 —save-peer # appends the new range
# would yield a "react": "~16.0 || ~16.1 || ~16.5" in package.json as result

# in that same package.json now forces a diff version
npm install react@~16.5 —save-peer -f # forces semver range
# would  yield a "react": "react": "~16.5" as a result

This type of support can be handy within a workspaces/monorepo setup allowing users to easily manage ranges for peer deps of multiple packages at once.

/cc @npm/cli-team

@ljharb
Copy link
Contributor

ljharb commented Jan 28, 2020

To clarify (since the starting state of the version range isn’t clear above), unlike save-prod/save-dev, which replace the version range, save-peer should add to it by default (it’s fine if something like -f replaces it), since it’s a semver-major change to exclude any previously valid version from a peer dep range.

@mikemimik mikemimik added the Agenda will be discussed at the Open RFC call label Feb 12, 2020
@ruyadorno
Copy link
Contributor Author

ruyadorno commented Mar 4, 2020

Notes from the OpenRFC call:

  • Do we want to have a --save-peer flag? YES, npm@7 will support it 👍
  • Have a different behavior when using the flag?
    • More unforeseen problems, how it relates to git repos, etc
    • No consensus on that yet
  • Let's follow up the discussion on how it interacts with workspaces in the upcoming deep-dive meeting: [POLL] Scheduling bi-weekly deep-dive (Wednesday, March 11th 2020) #105

@mikemimik
Copy link

Created https://github.com/npm/cli issue

@ruyadorno
Copy link
Contributor Author

We discussed this a bit more in the last OpenRFC call and decided to close this issue since --save-peer is already going to be available in npm@7 (which was the main theme here) - if anyone wants to submit specific RFCs on the different ways this config should behave please feel free to submit a RFC for that 😊

@isaacs
Copy link
Contributor

isaacs commented Apr 2, 2020

Specifically: the implementation as it currently stands in npm v7 is that --save-peer will follow the same save semantics as other --save* options. (Use the version resolved if --save-exact is set, otherwise prefix the resolved version with --save-prefix, which defaults to ^.)

If we want a different behavior for --save-peer, that's what needs an RFC.

@ljharb
Copy link
Contributor

ljharb commented Apr 2, 2020

That seems problematic given that the different behavior is the only desired “save peer” functionality, and means it will be delayed until v8.

Why will this be added in v7 when there’s already this issue roughly describing the actual need?

@isaacs
Copy link
Contributor

isaacs commented Apr 3, 2020

Well, v7 isn't finished yet (and even once it is "released", it'll be in beta for a few months at least), so there's plenty of time to change it. But if we want to change it, now's the time, and that change is what needs an RFC. (Depending on what that change looks like, it might not even need a breaking change, but that'd TBD of course.)

This isn't the first time someone's requested --save-peer, and it was already planned to come along with proper handling of peer deps. The part that's somewhat novel is saving it differently, which we're completely open to, but need to comb through the edge cases it'll introduce.

I'm just saying that "keep the current save semantics for save-peer" is the status quo path we're already on, so changing that path requires some discussion and evaluation.

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

5 participants