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
Error on yarn install if the lockfile is out of date #453
Conversation
Tests are currently failing because yarn fails on unknown command line flags, so I need to avoid passing in |
@@ -59,12 +59,28 @@ log_build_scripts() { | |||
fi | |||
} | |||
|
|||
yarn_supports_frozen_lockfile() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dzuelke Mind critiquing my bash here?
yarn_supports_frozen_lockfile() { | ||
local yarn_version="$(yarn --version)" | ||
# Yarn versions lower than 0.19 will crash if passed --frozen-lockfile | ||
if [[ "$yarn_version" =~ ^0\.(16|17|18).*$ ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List of all the yarn versions we support: https://nodebin.herokai.com/v1/yarn/linux-x64.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bash functions can return
exit codes like a normal program. So you can return 0
to indicate success, and any other code to indicate failure. Running if
against this then works without [
or [[
. You can also just use the true
and false
shorthands if they're the last statement, as the return code of a function will be its last statement. If you didn't need the mcount …
call, the last statement could thus also just be [[ ! "$yarn_version" =~ ^0\.(16|17|18).*$ ]]
lib/dependencies.sh
Outdated
# Yarn versions lower than 0.19 will crash if passed --frozen-lockfile | ||
if [[ "$yarn_version" =~ ^0\.(16|17|18).*$ ]]; then | ||
mcount "yarn.doesnt-support-frozen-lockfile" | ||
echo false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use return 1
, or just false
, without echo
lib/dependencies.sh
Outdated
mcount "yarn.doesnt-support-frozen-lockfile" | ||
echo false | ||
else | ||
echo true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use return 0
, or just true
, without echo
lib/dependencies.sh
Outdated
|
||
echo "Installing node modules (yarn.lock)" | ||
cd "$build_dir" | ||
yarn install --pure-lockfile --ignore-engines 2>&1 | ||
if [ "$supports_frozen_lockfile" = true ]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if yarn_supports_frozen_lockfile; then
(and, generally, always use [[
with if
if you require statements inside)
lib/dependencies.sh
Outdated
yarn_node_modules() { | ||
local build_dir=${1:-} | ||
local supports_frozen_lockfile=$(yarn_supports_frozen_lockfile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need that one; see below
I see how I could simplify to that, but I like the clarity of the more verbose option. Thanks for the insights @dzuelke! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just requested a tweak in the explanation text.
lib/failure.sh
Outdated
echo "" | ||
warn "Outdated Yarn lockfile | ||
|
||
Your application contains a Yarn lockfile: yarn.lock which does not match |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parentheses might fit the grammar here better than a colon:
Your application contains a Yarn lockfile; yarn.lock which does not match the requirements in your package.json file.
vs:
Your application contains a lockfile (yarn.lock) which does not match the dependencies in package.json.
updated wording 👍 @hunterloftis |
Thanks for getting this change in! sorry for not reviewing it sooner. |
Hello. |
@vlad-elagin hi, can you provide your |
Thanks for response! Here they are. |
I've got it working, thanks. |
This switches to using
--frozen-lockfile
for yarn installs, which will fail the build ifyarn.lock
does not match the requirements listed inpackage.json
. Under our current setup Yarn will install the updated dependencies as if the lockfile doesn't exist in this case, which could lead to unexpected failures at runtime.With this change it will fail the build and print out a helpful message in this case.
Example log and error message: