Use yarn install --frozen-lockfile in CI

[mmermerkaya]

From yarn documentation:

yarn install --frozen-lockfile
Don’t generate a yarn.lock lockfile and fail if an update is needed.

With this change, our CI build should fail during the install phase if the lock file wasn't updated properly. But it won't, because this doesn't actually work 😐. I guess if they ever fix this bug, it will start working. And then we can remove the integrity check from tools/pretest.js.

Also changes the fixed yarn version to latest, so that we won't have to change it manually after new yarn releases.

[yifanyang]

How about [email protected] or some other specific version?

We used to set to always download the latest version, but the problem with that is if a new yarn version gets released, we will not be aware of such update in our Travis build. And that has caused some trouble in the past. Also see #1351 and #1481.

[yifanyang]

LGTM.

[mmermerkaya]

Done. It would be the best if we could just get the yarn version from devDependencies (and actually add yarn to devDependencies), but I don't know how we can do that easily.

[yifanyang]

Agreed. The current way of managing yarn version isn't very pretty.

I guess the problem is that yarn <command> invokes the globally installed yarn. I don't know if we want to change our command to something like npx yarn <command>. And even with that we still need the globally installed yarn to run the first yarn install during a Travis build. I couldn't think of a good solution right now...

[mmermerkaya]

Yeah exactly what I'm thinking as well 😅

[Feiyang1]

Thanks for looking into this problem!
Instead of adding an option that doesn't work as of now, can we just add a script that checks if there are changes in the lock file after yarn install and fail if there were?

[mmermerkaya]

Before I opened this PR I had two assumptions:

• We don't have anything that checks the lock file (hence the need for Run yarn #1675).
• Changing the install command to yarn install --frozen-lockfile would fix this problem.

Turns out, both of my assumptions were wrong 😄 There is a script just like you mentioned here. I found out about this when I opened this PR and Travis failed with this error. It went away after I rebased this PR on top of #1675.

[Feiyang1]

I think we still need a separate script to check if yarn.lock is changed after yarn install using git diff. If there was any diff, it means yarn.lock was not up to date.
yarn check --integrity will pass after yarn install because it will update the lock file.

We already do it as part of the release process https://github.com/firebase/firebase-js-sdk/blob/master/scripts/release/utils/git.js#L78

And thanks again for looking into it. Let me know if you want me to do it.

[mmermerkaya]

No problem 🙂

yarn install --frozen-lockfile doesn't update the lock file and instead installs versions that are in the lock file, just like --pure-lockfile. I think --frozen-lockfile + integrity check should do the trick. I might be wrong though, the documentation isn't very clear.

[Feiyang1]

Sounds good as long as it works. Can you please add some comments? then I think we are good.

[Feiyang1]

Friendly ping. I ran into this problem again, I'd like to get this merged.

[mmermerkaya]

Copy pasted the yarn documentation.

[Feiyang1]

hmm, you mentioned it doesn't work the way it was documented, right? Can we just describe why we have it here, and how it works in its current state?
Something like:
--frozen-lockfile doesn’t generate a yarn.lock lockfile and fails if an update is needed. It will make sure that yarn.lock file in the PR is consistent with package.json. Due to a yarn issue (https://github.com/yarnpkg/yarn/issues/5840), --frozen-lockfile doesn't actually fail, but integrity check from tools/pretest.js will catch the issue, so it still works.

[mmermerkaya]

I see, done. I was hesitant to mention that it doesn't work as expected, because if they fix it, the comment will be wrong and that's worse than having no comment at all 😄. But I see the benefit of mentioning it here, in case someone else notices the same issue and wonders how this works.

I subscribed to the yarn issue. Hopefully I'll remember to change this comment if it ever gets fixed 🙂