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

fix: npm update --save #4223

Merged
merged 4 commits into from
Jan 20, 2022
Merged

Conversation

ruyadorno
Copy link
Contributor

@ruyadorno ruyadorno commented Jan 7, 2022

Previously npm update was not respecting the save option, it
would be impossible for users to use npm update and automatically
update their package.json files.

This fixes it by adding extra steps on Arborist.reify._saveIdealTree
to read direct dependencies of any package.json and update them as
needed when reifying using the update and save options.

Also the follow up changes make sure to integrate the original arborist fix
into the cli while better documenting these expectations:

  • Uses config.isDefault to set a different value for the save config
    for both the update and dedupe commands
  • Tweaks arborist to make sure saveIdealTree preserves the behavior of
    skipping writing to package-lock.json on save=false for install while
    still writing the lockfile for npm update with its new default value
    of save=false.
  • Updated and added some new tests on arborist to cover for these tweaks
  • Added npm update --save smoke test on cli

References

Fixes: #708
Fixes: #2704
Relates to: npm/feedback#270

@ruyadorno ruyadorno requested a review from a team as a code owner January 7, 2022 23:10
ruyadorno added a commit to ruyadorno/config that referenced this pull request Jan 10, 2022
Add a new method `isDefault(key)` that enables a way to know if the
value to be returned by a `config.get(key)` call is coming from the
default definitions or any other different source. In case it's coming
from the default values this method returns `true`, it returns `false`
otherwise.

This addition is going to allow for effectively managing different
default values in the npm cli on a command-basis, e.g:

The `save` config default value in the npm cli is `true`, so that in
`npm install` and other commands that value is always going to default
to true if no user config is provided. Now let's say we want to use a
different value in `npm update`, for example `save=false`. This change
enables us to have a conditional check to see if the `save` config value
is coming from the default source, thus providing a way to use a
different value instead of the default:

    const save = config.isDefault('save') ? false : config.get('save')

Relates to: npm/cli#4223
Relates to: npm/statusboard#324
ruyadorno added a commit to npm/config that referenced this pull request Jan 10, 2022
Add a new method `isDefault(key)` that enables a way to know if the
value to be returned by a `config.get(key)` call is coming from the
default definitions or any other different source. In case it's coming
from the default values this method returns `true`, it returns `false`
otherwise.

This addition is going to allow for effectively managing different
default values in the npm cli on a command-basis, e.g:

The `save` config default value in the npm cli is `true`, so that in
`npm install` and other commands that value is always going to default
to true if no user config is provided. Now let's say we want to use a
different value in `npm update`, for example `save=false`. This change
enables us to have a conditional check to see if the `save` config value
is coming from the default source, thus providing a way to use a
different value instead of the default:

    const save = config.isDefault('save') ? false : config.get('save')

Relates to: npm/cli#4223
Relates to: npm/statusboard#324
@darcyclarke darcyclarke added semver:patch semver patch level for changes Release 8.x work is associated with a specific npm 8 release cmd:update related to `npm update` release: next These items should be addressed in the next release labels Jan 11, 2022
Previously `npm update` was not respecting the `save` option, it
would be impossible for users to use `npm update` and automatically
update their `package.json` files.

This fixes it by adding extra steps on `Arborist.reify._saveIdealTree`
to read direct dependencies of any `package.json` and update them as
needed when reifying using the `update` and `save` options.

Fixes: npm#708
Fixes: npm#2704
Relates to: npm/feedback#270
- Uses config.isDefault to set a different value for the `save` config
  for both the update and dedupe commands
- Tweaks arborist to make sure saveIdealTree preserves the behavior of
  skipping writing to package-lock.json on save=false for install while
  still writing the lockfile for `npm update` with its new default value
  of save=false.
- Updated and added some new tests on arborist to cover for these tweaks
- Added `npm update --save` smoke test on cli
@ruyadorno ruyadorno requested review from fritzy and nlf January 12, 2022 03:49
@darcyclarke darcyclarke added the Agenda will be discussed at the Open RFC call label Jan 12, 2022
@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Jan 13, 2022
- added more tests scenarios
- fixed some typos
Copy link
Contributor

@darcyclarke darcyclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd:update related to `npm update` release: next These items should be addressed in the next release Release 8.x work is associated with a specific npm 8 release semver:patch semver patch level for changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants