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

Aborting publish should not change version number #662

Open
tkloht opened this issue Oct 11, 2016 · 4 comments
Open

Aborting publish should not change version number #662

tkloht opened this issue Oct 11, 2016 · 4 comments

Comments

@tkloht
Copy link
Contributor

tkloht commented Oct 11, 2016

Do you want to request a feature or report a bug?
bug
What is the current behavior?
When using yarn publish and aborting after entering a version number, the version number is not reset. As a consequence it seems impossible to publish the same version number "again" without reverting the change manually.
If the current behavior is a bug, please provide the steps to reproduce.

[tkloht@Tobias-Air ~/Code/react-component-scripts:master] yarn publish
yarn publish v0.15.1
[1/4] Bumping version...
info Current version: 1.4.1
question New version: 1.4.2
info New version: 1.4.2
[2/4] Logging in...
info npm username: tkloht
info npm username: tobias.kloht@gmail.com
error canceled
    at Interface.<anonymous> (/Users/tkloht/.nvm/versions/node/v6.2.1/lib/node_modules/yarnpkg/node_modules/read/lib/read.js:66:13)
    at emitNone (events.js:86:13)
    at Interface.emit (events.js:185:7)
    at Interface._ttyWrite (readline.js:707:16)
    at ReadStream.onkeypress (readline.js:115:10)
    at emitTwo (events.js:106:13)
    at ReadStream.emit (events.js:191:7)
    at emitKeys (internal/readline.js:385:14)
    at next (native)
    at ReadStream.onData (readline.js:939:36)
info Visit http://yarnpkg.com/en/docs/cli/publish for documentation about this command.

after this the version number is already changed:

diff --git a/package.json b/package.json
index e8764fc..c6d0fb4 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@tkloht/react-component-scripts",
-  "version": "1.4.1",
+  "version": "1.4.2",

This makes it impossible to publish version 1.4.2 in this example:

[tkloht@Tobias-Air ~/Code/react-component-scripts:master*] yarn publish
yarn publish v0.15.1
[1/4] Bumping version...
info Current version: 1.4.2
question New version: 1.4.2
error New version is the same as the current version.
info Visit http://yarnpkg.com/en/docs/cli/publish for documentation about this command.

What is the expected behavior?

When I abort a publish the version number in package.json should not have changed.

Please mention your node.js, yarn and operating system version.
node: v6.2.1, yarn: 0.15.1, OSX 10.12

@tkloht
Copy link
Contributor Author

tkloht commented Oct 12, 2016

I think I just figured out what the current behaviour actually is: When you abort after changing the version but before publish, the version is changed. Then when you publish again you are asked for the new version. When you enter a new version the cli runs a simple check, and if you enter the same version a gain exits with an error "New version is the same as the current version". But if you don't enter a number it continues with the version currently set in package.json. If the version is already published it fails in the next step. As far as I can tell this is the exact same behaviour of the npm cli (npm version [current-version-in-package.json] exits with error), so I'm not sure if and how this should be changed. I guess it would be possible to add another step of user interaction ("publish with same version? yes/no", something like that), or it might be possible to check against the registry if the new version is valid. Just some ideas, also thank you for the great work.

@bestander
Copy link
Member

We'll focus on a better publish experience soon

@pvdz
Copy link

pvdz commented Oct 11, 2017

This issue may seem easy to fix but it is harder than it seems. In case somebody tries to tackle it here are some things to consider:

  • The version needs to be bumped before publishing because the package.json file is sent to the registry as-is
  • We could move this bump to after the logging-in-to-npm step but it could still forcefully stop prematurely between the version bump and the actual publish (as late as the package registry rejecting the publish)
  • The internal setVersion call, which causes the actual update to the version in the file, will commit the version bump if it detects a git repo. You'll need to undo this commit as well, but also make sure you're not undoing a random commit or and consider things that may have happened in other hooks like prepublish, because that could severely affect data integrity
  • There is support for reading from an archive in this code so simply searching for, backing up, and restoring the package.json file is not trivial
    • and; If you're going to backup the package.json then why not back up and restore certain other files, or make a checkpoint to return
  • Since publish will also invoke hooks like prepublish, there could be many other modifications that are non-trivial to detect and restore. They may or may not affect package.json. How to deal with that.

The fix in question is going to be very limited or otherwise require quite a bit of change. Note that "limited" may make sense for your use case but could confuse people with even slightly more complex setups.

I suspect this will be a wontfix for now. I'm closing my PR, which was flawed anyways.

@bestander
Copy link
Member

@qfox, yeah this is a complex problem, publish is not an atomic operation with side effects around multiple systems.
Thanks for sharing your line of thought.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants