RFC: Add publish confirmation prompt #96

ruyadorno · 2020-01-31T20:41:26Z

npm publish should have a confirmation prompt in order to provide a chance for users to review the package contents, similar to how 2FA-enabled users are able to review the contents prior to inputting their OTP.

See RFC

/cc @mikemimik

ljharb · 2020-01-31T21:21:20Z

0018-publish-prompt.md

+
+## Implementation
+
+This is a breaking change from the current `npm publish` behavior, it would prompt the user for confirmation:


perhaps a good first step would be, provide an option to enable the prompt, and then change the default in a future major?

We could just make the default be --yes when process.stdout.isTTY === false. Then it'll keep on working for build scripts and CI, but prompt everyone on the CLI.

Do you have plans to align with other package managers? Would be great to have a consistent behavior, I think this is a great idea.

I agree, I think we would do this as well in Yarn. cc @zkochan

Seems fine for single-package scenario but I am not sure this would work during a publish in a monorepo. I doubt that someone would review the content of every published package if dozens are published.

What if it would only show the prompt if the list of files that are published differs from the list of files in the previously published version?

Not all monorepos publish multiple packages at once; in my experience many use independent versioning at this point.

Printing a file diff if possible could be interesting 🤔

Something else to consider is that workflows publishing multiple packages (which do exist, for example this is what we do on Yarn, and I think Babel and Jest do the same) might be more likely to just use --yes (which would be handy, but would kinda negate this RFC altogether).

multiple packages is common; "bumping all package versions at once even if they haven't changed, to keep all versions the same" is the original lerna behavior that babel, and many others, have thankfully abandoned :-)

Clarifying Lerna behavior for future reference:

"bumping all package versions at once even if they haven't changed, to keep all versions the same"

This isn't actually what Lerna does in the default "fixed" versioning scheme. It applies a single version to any package (and its local dependents) that has changed since the previous release (annotated git tag).

Not all monorepos publish multiple packages at once; in my experience many use independent versioning at this point.

"Independent" versioning doesn't change anything about how Lerna detects changes between releases or bumps dependents of changed packages, it merely allows one to choose versions per-package instead of globally.

0018-publish-prompt.md

ruyadorno · 2020-02-19T19:30:55Z

Notes from the OpenRFC call:

Let's make this an opt-in for npm@7 and then move it to opt-out in a next major release
The client has no knowledge on whether 2FA is enabled or not
2FA awareness could/should be addressed in different features / RFCs

ruyadorno · 2020-03-04T20:31:32Z

Updated contents of the RFC based on feedback collected during the OpenRFC call and the comments left here 😊 should be ready for ratification if no contention point is raised

accepted/0000-publish-prompt.md

Co-Authored-By: Jordan Harband <[email protected]>

…o/rfcs into add-publish-confirmation-prompt

ruyadorno · 2020-03-18T17:54:17Z

cleaned up the RFC and documented all feedback collected 👍 should be good to go

eridal · 2020-03-18T18:45:37Z

Is it possible to make the files listed in Tarball Contents be sorted alphabetically?

ruyadorno · 2020-03-18T22:08:34Z

@eridal that does sounds a bit outside of the scope of this RFC but that said, it would def be nice to have! 👍

- Changed opt-in flag name to remove reference to "experimental"

ruyadorno · 2020-03-18T22:11:48Z

Updated with feedback from the OpenRFC call removing "experimental" from the name of the option/flag.

eridal · 2020-03-18T22:32:51Z

@ruyadorno thanks for your answer

Does that happens here? https://github.com/npm/libnpmpack/blob/ebe5d4790f61f93c7045dd0341f61681ed520899/utils/tar.js#L18

I'd like to attempt to submit a separate PR to address that feature, but that module seems to be currently worked on

isaacs · 2020-03-19T00:20:01Z

@eridal Yes, that's currently being worked on by @claudiahdz. Publish and pack will both use the same code path for displaying the tarball contents, and presumably we'd leave that untouched by this RFC. Sorting should be straightforward, I'd think, but I recommend you post an issue on https://github.com/npm/libnpmpack, just so that there's a thing to track.

@ruyadorno

This will make `npm pack` output the tarball contents in an alphabetically and natural sort order. The current implementation uses `localeCompare` with the following options: 1. **sensitivity**: `base`, to correctly handle non-ascii letters using their base ascii, so that `á`,`â`, `ä`, `ã`, `å` are all handled like `a` 1. **numbers**: `true`, to handle numeric filenames as natural ascending order so that, for instance, file `10.txt` appears after `9.txt` --- Convo from npm/rfcs#96 cc @ruyadorno @isaacs

@ruyadorno

This will make `npm pack` output the tarball contents in an alphabetically and natural sort order. The current implementation uses `localeCompare` with the following options: 1. **sensitivity**: `base`, to correctly handle non-ascii letters using their base ascii, so that `á`,`â`, `ä`, `ã`, `å` are all handled like `a` 1. **numbers**: `true`, to handle numeric filenames as natural ascending order so that, for instance, file `10.txt` appears after `9.txt` --- Convo from npm/rfcs#96 cc @ruyadorno @isaacs

@ruyadorno

This will make `npm pack` output the tarball contents in an alphabetically and natural sort order. The current implementation uses `localeCompare` with the following options: 1. **sensitivity**: `base`, to correctly handle non-ascii letters using their base ascii, so that `á`,`â`, `ä`, `ã`, `å` are all handled like `a` 1. **numbers**: `true`, to handle numeric filenames as natural ascending order so that, for instance, file `10.txt` appears after `9.txt` --- Convo from npm/rfcs#96 cc @ruyadorno @isaacs

ruyadorno · 2020-07-22T18:47:04Z

Landed in 0ad3bdc

ruyadorno added the Agenda will be discussed at the Open RFC call label Jan 31, 2020

ruyadorno requested a review from a team January 31, 2020 20:41

ljharb reviewed Jan 31, 2020

View reviewed changes

This was referenced Feb 5, 2020

Open RFC Meeting - Wednesday, February 5, 2020, 2:00 PM EST #99

Closed

Open RFC Meeting - Wednesday, February 19, 2020, 2:00 PM EST #101

Closed

juanpicado reviewed Feb 19, 2020

View reviewed changes

0018-publish-prompt.md Outdated Show resolved Hide resolved

RFC: Add publish confirmation prompt

445af22

ruyadorno force-pushed the add-publish-confirmation-prompt branch from b1763f9 to 445af22 Compare February 24, 2020 22:21

darcyclarke mentioned this pull request Mar 2, 2020

Open RFC Meeting - Wednesday, March 4, 2020, 2:00 PM EST #104

Closed

darcyclarke added Needs Discussion is pending a discussion and removed Agenda will be discussed at the Open RFC call labels Mar 4, 2020

Review based on feedback from OpenRFC call and comments

299e813

ruyadorno added Agenda will be discussed at the Open RFC call and removed Needs Discussion is pending a discussion labels Mar 4, 2020

ljharb reviewed Mar 4, 2020

View reviewed changes

accepted/0000-publish-prompt.md Outdated Show resolved Hide resolved

accepted/0000-publish-prompt.md Outdated Show resolved Hide resolved

darcyclarke assigned ruyadorno Mar 10, 2020

darcyclarke added this to the OSS - Sprint 6 milestone Mar 10, 2020

Update accepted/0000-publish-prompt.md

14a81ef

Co-Authored-By: Jordan Harband <[email protected]>

darcyclarke mentioned this pull request Mar 17, 2020

Open RFC Meeting - Wednesday, March 18, 2020, 2:00 PM EST #111

Closed

ruyadorno added 2 commits March 18, 2020 13:52

Added non-tty and experimental flag example

825a74f

Merge branch 'add-publish-confirmation-prompt' of github.com:ruyadorn…

40c9895

…o/rfcs into add-publish-confirmation-prompt

darcyclarke removed the Agenda will be discussed at the Open RFC call label Mar 18, 2020

Feedback from OpenRFC call

8129a02

- Changed opt-in flag name to remove reference to "experimental"

eridal mentioned this pull request Mar 19, 2020

Sorted tarball contents npm/libnpmpack#1

Closed

darcyclarke modified the milestones: OSS - Sprint 6, OSS - Sprint 7 Apr 7, 2020

darcyclarke removed this from the OSS - Sprint 7 milestone Jun 1, 2020

ruyadorno added the Agenda will be discussed at the Open RFC call label Jul 15, 2020

This was referenced Jul 15, 2020

Open RFC Meeting - Wednesday, July 15, 2020, 2:00 PM EST #178

Closed

Open RFC Meeting - Wednesday, July 22, 2020, 2:00 PM EST #180

Closed

darcyclarke removed the Agenda will be discussed at the Open RFC call label Jul 22, 2020

ruyadorno closed this Jul 22, 2020

cactysman mentioned this pull request Jul 31, 2020

Add registry identifier to publish prompt. #191

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: Add publish confirmation prompt #96

RFC: Add publish confirmation prompt #96

ruyadorno commented Jan 31, 2020 •

edited

Loading

ljharb Jan 31, 2020

isaacs Feb 3, 2020

juanpicado Feb 4, 2020

arcanis Feb 4, 2020

zkochan Feb 4, 2020

zkochan Feb 4, 2020

ljharb Feb 4, 2020

arcanis Feb 4, 2020

ljharb Feb 19, 2020

evocateur Feb 24, 2020

ruyadorno commented Feb 19, 2020 •

edited

Loading

ruyadorno commented Mar 4, 2020

ruyadorno commented Mar 18, 2020

eridal commented Mar 18, 2020

ruyadorno commented Mar 18, 2020

ruyadorno commented Mar 18, 2020

eridal commented Mar 18, 2020

isaacs commented Mar 19, 2020

ruyadorno commented Jul 22, 2020


		## Implementation

		This is a breaking change from the current `npm publish` behavior, it would prompt the user for confirmation:

RFC: Add publish confirmation prompt #96

RFC: Add publish confirmation prompt #96

Conversation

ruyadorno commented Jan 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruyadorno commented Feb 19, 2020 • edited Loading

ruyadorno commented Mar 4, 2020

ruyadorno commented Mar 18, 2020

eridal commented Mar 18, 2020

ruyadorno commented Mar 18, 2020

ruyadorno commented Mar 18, 2020

eridal commented Mar 18, 2020

isaacs commented Mar 19, 2020

ruyadorno commented Jul 22, 2020

ruyadorno commented Jan 31, 2020 •

edited

Loading

ruyadorno commented Feb 19, 2020 •

edited

Loading