Problem/Motivation

Followup from #3262573-11: Update our yarn dev dependencies to the extent allowed by current constraints.

Currently, the commit check script has some very generic error messaging when yarn check -s exits with an error status:

# Ensure JavaScript development dependencies are installed.                     
yarn check -s 2>/dev/null
if [ "$?" -ne "0" ]; then
  printf "Drupal's JavaScript development dependencies are not installed. Run 'yarn install' inside the core directory.\n"
  DEPENDENCIES_NEED_INSTALLING=1;
fi

There are other reasons that yarn check -s can fail besides the dependencies not being installed. In particular, upstream issues or irresolvable conflicts can also cause the problem, e.g.:

[ayrton:core | Fri 17:31:33] $ yarn check -s
warning ../../../package.json: No license field
error "terser#acorn@^8.5.0" doesn't satisfy found match of "acorn@7.4.1"
warning "eslint#@babel/code-frame#@babel/highlight@^7.10.4" could be deduped from "7.16.10" to "@babel/highlight@7.16.10"
error Found 1 errors.

Proposed resolution

Update the error message text to include other possible causes and solutions.

Remaining tasks

TBD

User interface changes

A change to the CLI and testbot output when yarn fails (not UI per se).

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#2 yarn-3267705.patch702 bytesxjm

Comments

xjm created an issue. See original summary.

xjm’s picture

Status: Active » Needs review
StatusFileSize
new702 bytes

Status: Needs review » Needs work

The last submitted patch, 2: yarn-3267705.patch, failed testing. View results

longwave’s picture

+++ b/core/scripts/dev/commit-code-check.sh
@@ -157,7 +157,7 @@
 yarn check -s 2>/dev/null

Should we just remove 2>/dev/null from here instead? yarn check -s only prints output if there is a problem, so it would be useful to print it here if that is the case.

xjm’s picture

Status: Needs work » Needs review

Re: #4, if it in fact is the case that the yarn dependencies aren't installed, then yarn "helpfully" prints an error message for each missing dependency. All 1300 of them. So I think we probably don't want to do that. ;)

Status: Needs review » Needs work

The last submitted patch, 2: yarn-3267705.patch, failed testing. View results

xjm’s picture

Status: Needs work » Needs review

HEAD's a mess of random fails still, but the patch is NR.

Status: Needs review » Needs work

The last submitted patch, 2: yarn-3267705.patch, failed testing. View results

xjm’s picture

Status: Needs work » Needs review

srsly

longwave’s picture

Status: Needs review » Reviewed & tested by the community

The new message is an improvement, but it would be nice if we could detect the different cases and tell the user exactly what to do.

  • lauriii committed 78388a4 on 10.0.x
    Issue #3267705 by xjm, longwave: Fix error message when 'yarn check -s'...

  • lauriii committed d933e18 on 9.4.x
    Issue #3267705 by xjm, longwave: Fix error message when 'yarn check -s'...
lauriii’s picture

Status: Reviewed & tested by the community » Fixed

Committed 78388a4 and pushed to 10.0.x. Committed to 9.4.x and 9.3.x since this only impacts the CLI output of our dev tooling. Thanks!

  • lauriii committed e2ad80d on 9.3.x
    Issue #3267705 by xjm, longwave: Fix error message when 'yarn check -s'...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.