The yarn.lock File

[oversize]

I want to open this discussion to allow others to understand my intentions and have the possibility for others for feedback, since in the original PR that might not be done enough.

With the merge of #832 the yarn.lock file was commited to the repository again.

Why add the lockfile

The yarn.lock file holds an exact tree of the projects dependencies locked to a specific version. Commiting that lock file to the repository provides a way to have deterministic builds. Meaning everybody will end up with the same dependency tree as defined in the lockfile. The workflow added above wants to ensure that the current build is functioning. It therefore is crucial to have that deterministic build where everyone (including the workflow) ends up on the same page. That is every developer and also the workflow.

A much better explanation of whats going on under the hood can be found in the npm docs: https://docs.npmjs.com/cli/v6/configuring-npm/package-locks

Yarn Docs about why it should be included: https://classic.yarnpkg.com/blog/2016/11/24/lockfiles-for-all/

Existing branches should update

What should existing branches that have not yet added the yarn.lock file do? Ideally they should merge from main to have the lockfile. If not, they should not provide a lockfile because the main lockfile represents the "current state of dependencies".

Should branches have the need to update the lockfile, because they want to change some dependecy (see below) a new lockfile should be generated after the merge to main and commited to main to reflect the new dependecies.

When does the lockfile change?

The lockfile should never be changed by any PR unless it deliberately changes some of the projects dependencies (add/remove/update) or a version bump like this one #862

Should anyone that is unintentionally altering the dependencies end up with a change in his local lockfile, he almost always should revert that change and not commit it. See below "update docs"

There was a bug in yarn that would cause that to happen. yarnpkg/yarn#4379 But that should be fixed now. He should use the --frozen-lockfile option to yarn install.

Update the docs

The docs that currently say you should not commit the yarn.lock file should be updated.

If the yarn.lock file is in the repository it might very well be that the checkout cant happen because there is a local version already. That local version should be removed first to allow the checkout to happen. Should that break something in you local install, then the dependency update should be noted in the PR. See above for what to do if a PR changes dependencies.

To ensure that deterministic build behaviour, we should advise people to always use the --frozen-lockfile option to yarn install.

There is no option to ensure deterministic builds in yarn. The easiest option for devs seems to be the --pure-lockfile option since it will not change the existing yarn.lock. There is a discussion about ´--frozen-lockfile´ to be the default behaviour that gives some insights into this: yarnpkg/yarn#4147

@rphair regarding your question:

No Branch should just add its own yarn.lock file if the branch currently does not have one (as the revamp-stake-pool. It could either merge main (which would make sense anyhow since its 102 commits behind already) or wait until its merged into main without bringing in its own yarn.lock!

The yarn.lock of the main branch should always reflect the "current state of dependencies" everybody agreed upon.

Other projects with lockfiles

The concept of a lockfile is not unique to yarn/npm. Other projects use that as well and all recommend the lockfile to be commited for the exact same reason.

• composer (PHP)
• pipenv (Python)
• bundler (Ruby)

@rphair @rdlrt @fill-the-fill @katomm @os11k Not sure how active you guys look into the discussions so i marked you here to get your thoughts on this.

[rphair]

@oversize thanks for this... I'll keep using it as a reference this week, will ask for further clarification about any of the above it it seems unworkable or unclear, and then when happy with every detail I'll plan to "Update the docs" so it's explicit for both developers and contributors.

[oversize]

Update

After some more reading and playing around with yarn install i realized that

• yarn install will recalculate dependencies, even if there is a lockfile and then change the existing lockfile to reflect the new versions. Resulting in what was already discussed in another thread: A changed lockfile for no obvious reason.
• When given the --frozen-lockfile option it errors out that it wants to change it but can not due to the option.
• There is another option: --pure-lockfile that does not change the lockfile ... but installs new dependencies anyhow ...

So while having a lockfile, yarn seems to interpret its use differently from what i was used to (coming from other package managers with lockfiles. That is: It just does not bother at all that there is a lockfile, because it always will recalculate and install the versions it sees from package.json and then just update (or not) the lockfile.

There is some interesting discussion about this here: yarnpkg/yarn#5847 Especially this comment sums it up pretty well: yarnpkg/yarn#5847 (comment)

At this point i have no idea what the lockfile in yarn is good for then. My conclusion is to update the above docs (and our docs) so that the developers should always use --pure-lockfile. Which unfortunatly will not result in a deterministic build, but at least dont bother them whith lockfile changes that they might no be able to cope with.

Sidenote: In the npm world is a special command npm ci that does what i think a lockfile is for. Install all the dependecies exactly as given in the lockfile and do not reinterpret version ranges from package.json. It requires therefor a package-lock.json. There is no such thing in yarn and the devs have provided the above solutions / options ...

Another Sidenote: I am not a Frontend Developer. I have no real usage/exposure to yarn outside of this project. So all that i have written is from reading and playing around over the last days or so. If there is someone with better knowledge about how yarn/npm is supposed to work i am happy to learn how this should be dealt with.

[oversize]

The general idea of yarn seems to be that all uncertainty should be dealt with in the package.json. Meaning that the package.json should be able to provide enough clarity for the dependency resolver to end up with the same tree. Which obsoletes the idea of a lockfile entirely imho.

[rphair]

I hear you @oversize ... I'm a front end developer who has always used gulp instead of yarn so this is all new to me as well. I'll wait for changing any build documentation until we have more long time contributors chiming in.

One thing will help to clarify about your comment above is whether you suggest using --pure-lockfile in yarn build, yarn install, or both. Another reason this will not be deterministic is: not all contributors will use this option when doing their build, even if advised to. 🤪

For my own sake and that of developers working in countries distant from GitHub's and Node's CDN endpoints (e.g. India where I am), I would like to avoid doing a yarn install every time we're doing a local build. Having to check and/or download every dependency all over again can be very time-consuming due to the huge latency in these places.

From doing nearly 100 local builds at this point I've observed there are little consequences of non-determinism in the packages that are selected. Docusaurus has so far been completely tolerant of package differences except what I pointed out in #691... I believe a consequence of instability in Docusaurus during its beta period of version 2.

Finally, the last observation I can think of from all my local building: for a long time to come, there may be some PR's that have yarn.lock in them and others that don't. The worst (not very bad) consequence of this is that we may not be able to git checkout a branch without yarn.lock from a branch that does have it. I've gotten used to just deleting this file when necessary... but newer contributors would (and probably should) feel uneasy about doing this, and therefore it's an issue that would also have to be addressed in the documentation... at least temporarily, until we can verify all PR's and branches are following the same standard about yarn.lock.

cc @rdlrt @katomm

[katomm]

I want to references discussions #865 #829 #718 #665

[rdlrt]

I think I touched on this topic somewhere earlier but cannot find it. In my experience - it's good to have lock files committed to repository, but at the same time add those to .gitignore file. That's usually the only middle ground that satisfies both node and git standards for projects where node development isn't the primary use-case (in this case, this being an average builder wanting to add an entry or update doc)

Yarn/package lock files as described above are quite notorious for source code versioning wherein different machines can land up with very different order of packages and thus, differences in node_modules.

Advantage of having them added to .gitignore:

• A basic PR for addition of showcase/builder tool/update of documentation will not impact the lock files
• Those who really want/need to add an update (eg: audit fix, version update, new package) will still be able to opt in changes - reducing number of times the file will be updated unnecessarily.
• Having the file themselves in repo will ensure it's easier to replicate if required to troubleshoot builds

[rphair]

OK I'm replying mostly to #876 (comment) (directly above) but need some of these things to be considered independently.

This is timely (before Christmas holiday leave I hope) because I'm testing this actively with a PR I've just submitted (#898) in all which the advice above (which I agree with completely) has not been sufficient to avoid the deadlock in the local build process which I've reported before in the threads @katomm highlighted above.

Mainly, once yarn.lock gets added in a commit to a PR, I know no way to remove it from the PR without also removing it from the repository once that PR is committed.

You can see the difficulty caused by this with the commit history in #898: as of this moment, I've attempted to fix this by downloading the "canonical" yarn.lock from staging and pushed it from my local branch. Despite this, GitHub still reports a "huge diff" with yarn.lock: https://github.com/cardano-foundation/developer-portal/pull/898/files

So yes I do think for this & many other reasons, we need (unless I've missed something):

• yarn.lock added to .gitignore (of staging at least, and maybe other branches)
• .yarn also added to .gitignore, because at least some versions of yarn create .yarn/install-state.gz and people will inevitably check it in... this happened to me because Add yarn build workflow #832 suggested to me (maybe wrongly) that this file was part of the "yarn build workflow": https://github.com/cardano-foundation/developer-portal/commits/staging/.yarn
• yarn.lock left in the repository for staging (because it represents a production standard, unlike for branches like revamp-stake-pool-course)
• all contents of .yarn deleted from the repository for staging.

If everybody can agree, I will submit ASAP (or one of the "maintainers" if that's more proper) a PR that does these changes and hopefully we can push it through immediately. Otherwise anyone testing or submitting from a local branch will run into the problem that I'm currently having in #876.

What I reported in the threads that @katomm quoted is this "oscillation" state... resulting in inappropriately deleted / added / modified yarn.lock files... that local builds will run into when some of the local branches have yarn.lock in them and others don't. I believe (please correct me if I'm wrong) that the sooner yarn.lock is gitignore'd, the less likely this will be to stop people in their tracks.

On the remaining note: with yarn.lock gitignored, we should therefore not have to add --pure-lockfile to the contributor testing / installing instructions... because it would only be useful to collaborators / maintainers when creating & committing a new baseline yarn.lock, and even if documented on the front page we couldn't rely upon contributors to run yarn that way all the time.

cc @rdlrt @oversize

[rphair]

p.s. to the above, a main reason I marked this "ASAP"...

My own testing suggests that PR editors & contributors fixing this problem in individual PRs... by adding yarn.lock to .gitignore as suggested... will also check in those changes to .gitignore (because .gitignore itself isn't gitignored). Then one finds a deadlock condition of no longer being able to make the PR simply equal to staging + their desired changes. 😖

[rphair]

OK, based on the one thumbs-up from @rdlrt so far I've proposed these changes officially in #901. This is my best understanding of the discussion above & we'll see how it goes. 🤞 (no need to update documentation for contributors as I see it)