UIDATIMP-690 prevent ESLint from dying #712

zburke · 2020-10-11T12:32:58Z

Due to a bug in jsx-exlint, the changed line causes ESLint to die. The bug has been fixed but hasn't made its way into a release yet, and it would be really nice to have lint working again!

Refs UIDATIMP-690

Due to a [bug in jsx-exlint](jsx-eslint/jsx-ast-utils#103), the changed line causes ESLint to die. The bug [has been fixed](jsx-eslint/jsx-ast-utils#102) but hasn't made it's way into a release yet, and it would be really nice to have lint working again! Refs UIDATIMP-690

id-jenkins · 2020-10-11T12:35:02Z

yarn run v1.22.4 $ eslint .

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/FileExtensions/FileExtensionForm.js
97:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
98:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
99:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
100:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
101:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
102:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
103:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/MatchProfiles/MatchProfilesForm.js
189:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
190:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
191:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
192:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
193:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
194:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
195:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/MatchProfiles/ViewMatchProfile.js
206:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
207:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
208:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
209:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props

✖ 18 problems (18 errors, 0 warnings)
18 errors and 0 warnings potentially fixable with the --fix option.

info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

mariia-aloshyna · 2020-10-12T07:42:32Z

Hello @zburke ! Thank you for this fix, but why didn't this error occur in other PRs and locally?

zburke · 2020-10-12T11:11:40Z

@mariia-aloshyna, this error does occur locally for me, and you can see it in other PRs if you look at the console in Jenkins builds from last week and PRs that merged today (search for chainexpression).

This is the line from PR #695 that causes ESLint to fail, but it fails in such a polite and quiet way that I think nobody noticed at the time because on GitHub it looks like a comment, rather than an error:

yarn run v1.22.4
$ eslint .
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

But there are a few things to note:

In previous PRs, ESLint doesn't comment at all if it exits successfully (e.g. UIDATIMP-630: Refine an identifier matching for Instances #694, Update translation strings as of 20200928T202231 #693, ...) so the fact that it is commenting at all is a red flag.
In subsequent PRs, ESLint continues to make this innocent-looking comment (UIDATIMP-675: Job profile create-edit screen: change unusable options to disabled #714, UIDATIMP-678: Field mapping profile create-edit screen for instances: add sentence #715), but if you look at the output in Jenkins, as above, you will see the full error message:

The prop value with an expression type of ChainExpression could not be resolved. Please file issue to get this fixed immediately.
TypeError: Cannot read property 'range' of null
Occurred while linting /home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/src/settings/MappingProfiles/detailsSections/edit/ItemDetailSection/LoanAndAvailability.js:59
    at SourceCode.getTokenBefore (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/source-code/token-store/index.js:298:18)
    at checkSpacingBefore (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/rules/template-curly-spacing.js:60:42)
    at TemplateElement (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/rules/template-curly-spacing.js:119:17)
    at /home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/linter/safe-emitter.js:45:58
    at Array.forEach (<anonymous>)
    at Object.emit (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/linter/safe-emitter.js:45:38)
    at NodeEventGenerator.applySelector (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/linter/node-event-generator.js:254:26)
    at NodeEventGenerator.applySelectors (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/linter/node-event-generator.js:283:22)
    at NodeEventGenerator.enterNode (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/linter/node-event-generator.js:297:14)
    at CodePathAnalyzer.enterNode (/home/jenkins/workspace/folio-org_ui-data-import_PR-714/project/node_modules/eslint/lib/linter/code-path-analysis/code-path-analyzer.js:634:23)
error Command failed with exit code 2.

To be clear, there is nothing technically wrong with this line (the bug is in ESLint, not our own code) but because this line causes ESLint to abort before it finishes its job, that allowed UIDATAIMP-689 to slip through undetected. With this change in place but the one from UIDATAIMP-689 (PR #711) reverted, I now see that import error locally:

$ yarn lint
yarn run v1.21.1
$ eslint .
The prop value with an expression type of ChainExpression could not be resolved. Please file issue to get this fixed immediately.

Error while parsing /Users/zburke/temp/ui-data-import/src/settings/MatchProfiles/MatchProfilesForm.js
Line 38, column 10: Line 38: Identifier 'MatchingFieldsManager' has already been declared

  36 |   MatchingFieldsManager,
  37 | } from '../../components';
> 38 | import { MatchingFieldsManager } from '../../components/MatchingFieldsManager';
     |          ^
  39 | 
  40 | import { MatchCriterion } from '../../components/MatchCriterion/edit';
  41 | 
`parseForESLint` from parser `/Users/zburke/temp/ui-data-import/node_modules/babel-eslint/lib/index.js` is invalid and will just be ignored

/Users/zburke/temp/ui-data-import/src/settings/MatchProfiles/MatchProfiles.js
  38:35  error  Parse errors in imported module './MatchProfilesForm': Line 38: Identifier 'MatchingFieldsManager' has already been declared

  36 |   MatchingFieldsManager,
  37 | } from '../../components';
> 38 | import { MatchingFieldsManager } from '../../components/MatchingFieldsManager';
     |          ^
  39 | 
  40 | import { MatchCriterion } from '../../components/MatchCriterion/edit';
  41 |  (38:10)  import/named

/Users/zburke/temp/ui-data-import/src/settings/MatchProfiles/MatchProfilesForm.js
  38:10  error  Parsing error: Identifier 'MatchingFieldsManager' has already been declared

  36 |   MatchingFieldsManager,
  37 | } from '../../components';
> 38 | import { MatchingFieldsManager } from '../../components/MatchingFieldsManager';
     |          ^
  39 | 
  40 | import { MatchCriterion } from '../../components/MatchCriterion/edit';
  41 | 

✖ 2 problems (2 errors, 0 warnings)

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Maybe the problem is that Jenkins only listens for exit code 1 rather than anything non-zero?

id-jenkins · 2020-10-12T11:18:26Z

yarn run v1.22.4 $ eslint .

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/FileExtensions/FileExtensionForm.js
97:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
98:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
99:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
100:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
101:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
102:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
103:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/MatchProfiles/MatchProfilesForm.js
189:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
190:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
191:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
192:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
193:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
194:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
195:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/MatchProfiles/ViewMatchProfile.js
206:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
207:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
208:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
209:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props

✖ 18 problems (18 errors, 0 warnings)
18 errors and 0 warnings potentially fixable with the --fix option.

info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

zburke · 2020-10-12T11:26:58Z

Maybe the problem is that Jenkins only listens for exit code 1 rather than anything non-zero?

Ohhhhh, I see now: when ESLint finds lint errors it exits non-zero and reports those errors on stdout, which is in turn reported as a comment on the PR. What happened here is that ESLint itself choked and it reported that error on stderr, so we get the innocent-looking ESLint ran... comment on the PR but only see the details of the failure when we look at the full console output in Jenkins.

id-jenkins · 2020-10-12T11:29:24Z

yarn run v1.22.4 $ eslint .

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/FileExtensions/FileExtensionForm.js
97:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
98:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
99:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
100:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
101:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
102:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
103:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/MatchProfiles/MatchProfilesForm.js
189:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
190:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
191:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
192:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
193:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
194:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props
195:7 error Expected indentation of 8 space characters but found 6 react/jsx-indent-props

/home/jenkins/workspace/folio-org_ui-data-import_PR-712/project/src/settings/MatchProfiles/ViewMatchProfile.js
206:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
207:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
208:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props
209:9 error Expected indentation of 10 space characters but found 8 react/jsx-indent-props

✖ 18 problems (18 errors, 0 warnings)
18 errors and 0 warnings potentially fixable with the --fix option.

info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

sonarcloud · 2020-10-12T11:31:00Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities (and 0 Security Hotspots to review)
0 Code Smells

0.0% Coverage
0.0% Duplication

zburke requested review from mariia-aloshyna and IvanKryzhanovskyi October 11, 2020 12:32

mariia-aloshyna approved these changes Oct 12, 2020

View reviewed changes

IvanKryzhanovskyi approved these changes Oct 12, 2020

View reviewed changes

Merge branch 'master' into UIDATIMP-690

6a6d3cf

Merge branch 'master' into UIDATIMP-690

ae5719c

zburke merged commit 82d0edb into master Oct 12, 2020

zburke deleted the UIDATIMP-690 branch October 12, 2020 11:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UIDATIMP-690 prevent ESLint from dying #712

UIDATIMP-690 prevent ESLint from dying #712

zburke commented Oct 11, 2020

id-jenkins commented Oct 11, 2020

mariia-aloshyna commented Oct 12, 2020

zburke commented Oct 12, 2020

id-jenkins commented Oct 12, 2020

zburke commented Oct 12, 2020

id-jenkins commented Oct 12, 2020

sonarcloud bot commented Oct 12, 2020

UIDATIMP-690 prevent ESLint from dying #712

UIDATIMP-690 prevent ESLint from dying #712

Conversation

zburke commented Oct 11, 2020

id-jenkins commented Oct 11, 2020

mariia-aloshyna commented Oct 12, 2020

zburke commented Oct 12, 2020

id-jenkins commented Oct 12, 2020

zburke commented Oct 12, 2020

id-jenkins commented Oct 12, 2020

sonarcloud bot commented Oct 12, 2020