Fix/cleanup yarn lock

[scinos]

Changes proposed in this Pull Request

• Adds CI step to Lint job to validate that yarn.lock does not have uncommitted changes.
• Cleans up existing uncommitted changes in yarn.lock
• Removed --frozen-lockfile since it doesn't do what we expect (The --frozen-lockfile command-line option does not work yarnpkg/yarn#3313 Clearer documentation around yarn install / --frozen-lockfile / --pure-lockfile yarnpkg/yarn#5847) to avoid any future confusion

From https://app.circleci.com/pipelines/github/Automattic/wp-calypso/58032/workflows/819cbf5a-ec40-4989-a89a-c57186712b9d/jobs/715945/steps

Testing instructions

1. Checkout the branch, delete yarn.lock and run yarn. Run this script in the console and verify it shows the error:

DIRTY_FILES=$(git status --porcelain 2>/dev/null)
if [[ ! -z "$DIRTY_FILES" ]]; then
  echo "Repository contains uncommitted changes: "
  echo "$DIRTY_FILES"
  echo "You may need to checkout the branch, run 'yarn' and commit those files."
fi

2. Revert local changes to yarn.lock and run the script again, you shouldn't see any error.

[matticbot]

Test live: https://calypso.live/?branch=fix/cleanup-yarn-lock

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[simison]

I'm still confused (only skimmed linked issues); is there no way to prevent updates in package.json that are missing from lockfile or other way around? cc @kraftbj @jeherve since I contributed Automattic/jetpack#14640

[scinos]

is there no way to prevent updates in package.json that are missing from lockfile or other way around?

As far as I know, there is no way to do that using yarn flags. There are a few examples on how to do it "externally" (check that yarn.lock has not changed) in yarnpkg/yarn#5840

I did something similar in the CI config, now at least at test will fail if yarn.lock needs to be updated for any reason, for example because it no longer matches package.json

[sirreal]

Should we be removing --frozen-lockfile everywhere? It does seem like desirable behavior (if confusing):

If you need reproducible dependencies, which is usually the case with the continuous integration systems, you should pass --frozen-lockfile flag.
…
yarn install --frozen-lockfile
Don’t generate a yarn.lock lockfile and fail if an update is needed.

Do we have any theories on how we get into a state where the lockfile is out of date but not committed? I have a hard time imagining yarn.lock is modified locally but isn't committed.

[scinos]

IMO, the behaviour of --frozen-lockfile is broken, as it doesn't do what the doc says (build does not fail if an update of yarn.lock is needed). If we leave it around it can cause confusion and suggest the check for a dirty lockfile in CI is not needed.

My theories of why yarn.lock is not committed are:

1. People may have it in their local .gitignore
2. If you do cd apps/full-site-editing && npm add ..., yarn.lock is not updated at all. Although in this case you'll end up with a package-lock.json.
3. Maybe people are adding deps to package.json manually?

[scinos]

Extracted the commit to fix yarn.lock to #42041

[simison]

Incidentally, I just ended up with a change in lockfile by running yarn dev --sync in apps/full-site-editing:

I'd expect not to have these pop up while working.

[scinos]

@simison

Incidentally, I just ended up with a change in lockfile

In this branch? I can't reproduce it in commit 1b936b2a81 (before the rebase) or 6fb03ad5b7 (after the rebase)

[simison]

In this branch?

Sorry, I didn't specify. At #41966

Actually! I might've ran yarn --sync first accidentally, instead of yarn dev --sync.

[scinos]

Sorry, I didn't specify. At #41966

I get the same diff in yarn.lock.

Looks like that branch is not updated with master. It is using a yarn.lock that is "dirty" as well :(

if --frozen-lockfile is the only discussion point I'll be more than happy to undo those changes and just commit the new CI step to avoid the bleeding of uncommitted yarn.lock files.

[griffbrad]

Yeah. Let's just drop the --frozen-lockfile change from this PR so we can get the CI check in place.