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

(discussion) upgrade vs upgrade [package] version inconsistency #3603

Closed
rally25rs opened this issue Jun 7, 2017 · 11 comments
Closed

(discussion) upgrade vs upgrade [package] version inconsistency #3603

rally25rs opened this issue Jun 7, 2017 · 11 comments

Comments

@rally25rs
Copy link
Contributor

rally25rs commented Jun 7, 2017

this is a discussion issue spawned from #3510

Existing Behavior

Currently (yarn 0.24 & 0.25) the upgrade command has this behavior:

Given a package.json

"dependencies": {
    "left-pad": "<=1.1.1"
}

And the left-pad package has these versions defined in the registry:

0.0.9
1.0.0
1.1.0
1.1.1
1.1.2
1.1.3
2.0.0

yarn upgrade
Upgrades to 1.1.1 because it matches the package.json semver range (<=1.1.1)

yarn upgrade left-pad
Upgrades to 2.0.0 because a package was passed with no explicit version

yarn upgrade left-pad@~1.1.2
Upgrades to 1.1.3 because it is the latest match for the passed in semver range (~1.1.2)

This actually follows correctly with how the documentation for upgrade is currently written:

yarn upgrade

This command updates all dependencies to their latest version based on the version range specified in the package.json file.

yarn upgrade [package]

This upgrades a single named package to the version specified by the latest tag (potentially upgrading the package across major versions).

yarn upgrade [package@version]

This will upgrade (or downgrade) an installed package to the specified version. You can use any SemVer version number or range.

The current behavior happens because underneath upgrade it hands the parameters off to the add command, so it basically is the same as yarn add left-pad.
The add command treats the absence of an explicit version range as "use latest"

Up For Discussion

Should we make a breaking change to yarn upgrade [package] and split it into the following cases:

  • Leave upgrade with no additional parameters how it is:

yarn upgrade

This command updates all dependencies to their latest version based on the version range specified in the package.json file.

  • Change passing a package without an explicit version to respect package.json

yarn upgrade [package]

This upgrades a single named package to the latest version based on the version range specified in the package.json file.

  • Leave handling an explicit version the same as how it is

yarn upgrade [package@version]

This will upgrade (or downgrade) an installed package to the specified version. You can use any SemVer version number or range.

yarn upgrade --latest

This command updates all dependencies to the version specified by the latest tag (potentially upgrading the package across major versions).

yarn upgrade [package] --latest

This upgrades a single named package to the version specified by the latest tag (potentially upgrading the package across major versions).

@arcanis
Copy link
Member

arcanis commented Jun 8, 2017

My opinion is that we should have the following behaviors:

command no arg --latest
upgrade respect package.json ignore package.json
upgrade <name> respect package.json ignore package.json

instead of the current:

command no arg --latest
upgrade respect package.json ignore package.json
upgrade <name> ignore package.json ignore package.json

It won't be a breaking change: we'll just be more conservative than what we're doing now, not less.

@cpojer
Copy link
Contributor

cpojer commented Jun 8, 2017

fwiw I agree with @arcanis.

@rally25rs
Copy link
Contributor Author

Additional feature - Preserve version ranges:

Related to #2367 and #3609 there have been requests that upgrade and upgrade-interactive when upgrading to a new major version will preserve the package.json specified version range, if it exists.

So for example if package.json specifies the dependency

  "foo": "~0.1.2",
  "bar": "^1.2.3",
  "baz": "2.3.4"

Then if upgrade --latest jumps to a new major version, it will preserve the range specifiers, and upgrade to something like:

  "foo": "~5.0.0",
  "bar": "^6.0.0",
  "baz": "7.0.0"

I think this could be a bit of a can of worms though, because you can have goofy things in there:

  "a": "<1.2.3",
  "b": ">1.0.0 && <2.0.0",
  "c": "1.x || >=2.5.0 || 5.0.0 - 7.2.3"

How do you resolve these to some new "latest" and preserve the range?


Alternatively, we could reuse the existing flags that upgrade-interactive provides:

  • [--tilde/-T]
  • [--exact/-E]

and add them to the regular upgrade command. This would be much easier to implement than handling all the edge cases of crazy semver ranges that could exist in package.json, and would at least give some control over the range specifier. It might be a good-enough first step.

@rally25rs
Copy link
Contributor Author

rally25rs commented Jun 8, 2017

Additional feature - Make upgrade-interactive follow the same version ranges

It is also confusing that upgrade-interactive always ignores package.json and just checks latest.

It seems like upgrade-interactive should just be an "interactive" version of upgrade and follow the same version resolution. The --latest flag could be implemented there to cross major versions.

In other words:

command no arg --latest
upgrade respect package.json ignore package.json
upgrade <name> respect package.json ignore package.json
upgrade-interactive respect package.json ignore package.json

This would make both commands consistent in behavior.


If we do this, then the --tilde and --exact flags should only do anything when --latest is specified (because otherwise we aren't changing the range in package.json)

Additionally, these 2 flags would override the "preserve version range specifier" behavior mentioned above, because you have explicitly defined a new version range to use. Which means we should also add a --caret flag.

@arcanis
Copy link
Member

arcanis commented Jun 8, 2017

I think this could be a bit of a can of worms though, because you can have goofy things in there:

I think that if we choose to support this, we can just support the 99% case where the reference matches ^[>~^]{version}$. For all other references, we can just fallback to transforming them into {reference} || {new-version}, possibly writing a warning so that the user knows that they should probably take a look at this and fix the ranges themselves. Maybe we should even prevent them from being updated unless the user specifies an extra flag (--force?).

@kaylie-alexa
Copy link
Member

Thanks for the --latest flag @rally25rs, I think the consistency between yarn upgrade and yarn upgrade [name] clarifies a lot of things!

This may be beyond the scope (pun not intended :), but what do you guys think about making upgrade-interactive actually be able to choose the scope and the version, instead of current behavior

command current behavior
upgrade-interactive choose to update outdated packages to latest minor versions
upgrade-interactive --tilde choose to update outdated packages to latest major versions
upgrade-interactive --exact choose to update outdated packages to latest exact versions

Proposed behavior

  • Remove --tilde, --exact flags
  • upgrade-interactive lists all the packages that have new versions, and prints them with current scope, e.g current "foo": "~5.0.0"
  • For each package, give an option for each version "~6.0.0", "^6.0.0", "6.0.0", or maybe even an option to enter the version themselves.

The benefits I see about this approach

  • user may not always know if they want to bump all their packages to minor/major/exact -- it may depend on respecting the current scope of the package
  • user always knows what the final version looks like for exotic ranges, and thus doesn't ever need to manually edit the versions in package.json.
  • it only takes 1 command to upgrade all packages (as opposed to having to run the same command with different flags)

@cpojer
Copy link
Contributor

cpojer commented Jun 8, 2017

Hey @kaylieEB! We really appreciate your contributions to Yarn recently. We have a #development channel on our discord server – would you like to join us there for discussions? We'd love to have you.

See https://discordapp.com/invite/yarnpkg

@rally25rs
Copy link
Contributor Author

Here is an interesting tidbit for discussion:

Let's say you have in package.json:

"dependencies": {
  "left-pad": "^1.0.0"
}

and let's say you currently have left-pad 1.0.0 installed, but latest according to the registry is 1.1.3.

Currently, if you yarn upgrade then 1.1.3 is installed but your package.json is not modified:

"dependencies": {
  "left-pad": "^1.0.0"
}

If you yarn upgrade left-pad then your package.json is modified to match latest:

"dependencies": {
  "left-pad": "^1.1.3"
}

Moving forward, what do you think we should do? My opinion is that upgrade and upgrade <package> should not update your package.json (unless you have specified the --latest flag). The lock file would be modified to set the current installed version to 1.1.3, but the semver pattern would remain ^1.0.0

Anyone disagree?

@arcanis
Copy link
Member

arcanis commented Jun 12, 2017

That seems reasonable, yeah 👍

@bestander
Copy link
Member

@rally25rs what would be your next step?
Do you want to copy your issue into an RFC and send a PR as a followup?

I think all the points above make sense, would be great to get the commands consistent.

@rally25rs
Copy link
Contributor Author

Closing this issue.
Opened RFC PR: yarnpkg/rfcs#71

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

No branches or pull requests

6 participants