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

beachball not updating package-lock.json version field when publishing #525

Closed
matthias-ccri opened this issue May 26, 2021 · 10 comments · Fixed by #675
Closed

beachball not updating package-lock.json version field when publishing #525

matthias-ccri opened this issue May 26, 2021 · 10 comments · Fixed by #675
Labels
enhancement New feature or request

Comments

@matthias-ccri
Copy link

Hello,

I am using beachball to manage a single-package repo (not a monorepo), which it says on the beachball site: "single or monorepo". I'm using npm. I understand that the majority of monorepo users are using yarn, but npm is popular in the single repo ecosystem. Also, the best practice for npm is to use lockfiles (package-lock.json).

The issue is that package-lock.json files have a version field that corresponds to the version field of package.json, but beachball does not update this field in package-lock.json. Other tooling does, such as the npm version command.

Currently our workaround is to do some manual script wrangling with git:

# Beachball doesn't update package-lock.json files when publishing.
- npx beachball publish --yes --no-push
- npm i --package-lock-only
- git add -A
- git commit -m "applying package updates [skip ci]"
- git push --set-upstream origin main

This is a bit uglier than the happy path:

npx beachball publish --yes --message "applying package updates [skip ci]"

This related issue (#482) saw the same issue and added a postpublish hook, but it's not clear how to use it.

I do use beachball with yarn in other monorepos and it's wonderful. It would just be nice if beachball updated the lockfile. Would this be hard?

@ecraig12345
Copy link
Member

Sorry for the late response. You use the postpublish hook in a beachball.config.js:

module.exports = {
  // ... your other config ...
  hooks: {
    postpublish: async (packagePath, name, version) => {
      // your logic here
    }
  }
}

It might be possible to detect the package manager based on which type of lock file is present and automatically run an appropriate command, but I'm not sure offhand exactly what would be involved with implementing that.

@rajsite
Copy link

rajsite commented Jul 30, 2021

I tried to do something similar to what @matthias-ccri described in: #525 (comment)

But for an npm 7 workspace monorepo. It didn't actually work because beachball deletes the publish branch so the npm i --package-lock-only failed to find the expected package versions.

To use the approach @matthias-ccri used I'd need a way to skip deleting the publish branch for a npm workspace monorepo. I haven't tried the lifecycle hook @ecraig12345 described but as long as the timing is right, ie after package file updates but before publish branch delete, that might work out.

In addition native npm 7 monorepo workspace support in beachball would be nice. I'd expect usage of npm's native workspace support to be increasingly popular :)

@rajsite
Copy link

rajsite commented Aug 6, 2021

We were able to get the package-lock.json to update using a workflow similar to the following:

# Publish like normal
npx beachball publish --yes
# Pull in beachball's new changes
git merge --ff-only origin/main
# Delete the current node_modules, it seems to get in a state 
# after beachball publish that causes subsequent `npm install`s
# to fail with a npm 7 workspace monorepo
rm -rf node_modules
# Update just the lock file
npm i --package-lock-only
# Create and publish the commit only if there are lock file changes
git add -A
if git diff-index --quiet HEAD;
then
  echo "no changes found to push to main"
else
  echo "changes found to push to main"
  git commit -m "applying package lock updates [skip ci]"
  git push --set-upstream origin main
fi

The end result isn't perfect, we end up with two commits, one for the package publish and one for the lock file publish. It's also not as robust as native beachball support with the retries, etc. Native support for npm lock files + npm 7 workspaces would be awesome 🔥

Edit: Another big con of this approach is that it can cause unexpected version bumps for unrelated packages when the lock file is updated this way. I found a commit that revved several packages unexpectedly and because the commit is tagged [skip ci] was merged in without a validating build which silently broke main for future PRs.

@rajsite
Copy link

rajsite commented Dec 9, 2021

The workaround to run npm i --package-lock-only to update the lock file has been a bit flaky for us.

I came up with an alternative that uses the newer postbump hook (since beachball 2.20.0) and does string replacements inside the package-lock.json instead.

I don't think it's a great long-term solution since it's not updating the lockfile using context aware tooling, it's just doing dumb string replacements. We have got a few weeks of runway out of it so thought I'd share: https://gist.github.com/rajsite/71fb9a1bd32139c38e8b880a3618c020

@codyzu
Copy link

codyzu commented Apr 11, 2022

Combining the approaches from @rajsite and @ecraig12345 answers, we got this working by adding the following to our beachball.config.js file in the root of our monorepo:

const { execFileSync } = require('child_process')

module.exports = {
  // ...
  hooks: {
    postbump: (_, name) => {
      console.log(
        `${name} bumped, running 'npm install --package-lock-only --ignore-scripts' to update package-lock.json`
      )
      execFileSync('npm', ['install', '--package-lock-only', '--ignore-scripts'])
    }
  }
}

By running the npm install in the postbump hook, the modified package-lock.json will automatically be added to the bump commit & tag made by beachball. So far this just works 😎

@ecraig12345 would it be helpful to add in the docs

postbump?: (packagePath: string, name: string, version: string) => void | Promise<void>;
that modified files will be committed automatically in the postbump hook (unlike the postpublish hook)?

@rajsite
Copy link

rajsite commented Apr 13, 2022

By running the npm install in the postbump hook, the modified package-lock.json will automatically be added to the bump commit & tag made by beachball. So far this just works 😎

My biggest concern with that approach is looking back through the lockfile updates I found some instances where unrelated packages were updated after the npm i --package-lock-only. It was concerning because this is running at the very end of all build validation and pushed directly to main. It caused our main branch to be in a broken state where all new syncs and opened PRs started failing due to an unrelated dependency that was updated.

That's why we switched to an approach like the gist in the previous comment: #525 (comment)

@rajsite
Copy link

rajsite commented Apr 13, 2022

It looks like the the latest npm v8.6 was updated to reify the lockfile when npm version runs: npm/cli#4588
Right now beachball manually revs the version field in package.json instead of using the npm version command so it wouldn't take advantage of that.

@codyzu
Copy link

codyzu commented Apr 14, 2022

@rajsite seems odd that other packages would get updated (and break your setup). According to the docs and here, it should just regenerate the package-lock.json based on the versions in the package.json. Granted it's not that well documented. Any idea why your package.json and package-lock.json might have been out of sync??? Could that issue have been related to something else?

In any case, I'm going to watch this closely 👀

@codyzu
Copy link

codyzu commented Apr 14, 2022

It looks like the the latest npm v8.6 was updated to reify the lockfile when npm version runs: npm/cli#4588 Right now beachball manually revs the version field in package.json instead of using the npm version command so it wouldn't take advantage of that.

That would be a cleaner solution for sure, plus we would get all of the standard pre/post version script goodness that's standard in npm. According to #482 (comment) they are only using beachball with yarn, so I'm guessing we won't get much love on npm support 😭

I have a few ideas:

  1. beachball could detect if it's yarn or npm and use the correct tool to version (hard)
  2. beachball could provide a config option that actually does the bump, something like bumpScript where we could set it to npm version --no-git-tag-version ${version} or yarn version --no-git-tag-version ${version}. If it's not set, it would do the current behavior of just modifying the appropriate package.json files. Seems like that has the potential to simplify a lot of stuff and be a robust solution 🤔
  3. a config setting that you set the package manager, something like packageManager that could be set to npm, yarnclassic, yarn2, etc... That might be a nicer developer experience 🤷

@rajsite
Copy link

rajsite commented Apr 15, 2022

@rajsite seems odd that other packages would get updated (and break your setup). According to the docs and here, it should just regenerate the package-lock.json based on the versions in the package.json. Granted it's not that well documented. Any idea why your package.json and package-lock.json might have been out of sync??? Could that issue have been related to something else?

In any case, I'm going to watch this closely 👀

It was partially because my original script did the following:

# Delete the current node_modules, it seems to get in a state 
# after beachball publish that causes subsequent `npm install`s
# to fail with a npm 7 workspace monorepo
rm -rf node_modules
# Update just the lock file
npm i --package-lock-only

So my original script deleted the node_modules folder before regenerating the lock file. When it does that all the transitive dependencies of your application are redownloaded and the lock file is created with new versions.

If you don't delete the node_modules folder then npm install --package-lock-only --ignore-scripts will try and create a lock file using the current node_modules structure. While it is unlikely that build resulted in changes (for example parcel will add new packages automatically) or that packages were deprecated during that time there is no guarantee that the npm install won't result in new or unexpected packages being added due to transitive dependencies.

rajsite pushed a commit to ni/javascript-styleguide that referenced this issue Oct 4, 2022
# Justification

We were using the `@ni/beachball-lock-update` package to work around [a
bug](microsoft/beachball#525) with how
beachball modifies `package-lock.json` when publishing. That bug has
been addressed in more recent versions of beachball so we can stop using
the workaround.

This moves us towards fixing [this Nimble
issue](ni/nimble#601).

# Implementation

1. Remove `@ni/beachball-lock-update` from `package.json` and
`beachball.config.js`.
2. Install the latest version of `beachball`

# Testing

1. We've been running with this setup in Nimble for a few months and
it's worked well
2. I verified that basic beachball commands still work
3. I'll monitor the publishing pipeline as future changes go in. I
considered making a trivial edit to a pakckage to trigger a publish but
that seems pretty noisy now that our apps uptake new versions every
night.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants