Problem/Motivation

ESLint was failing with the following errors:

$ $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml . || true
Oops! Something went wrong! :(
ESLint: 8.55.0
ESLint couldn't find the config "airbnb-base" to extend from. Please check that the name of the config is correct.
The config "airbnb-base" was referenced from the config file in "/builds/issue/project_browser-3415079/web/modules/custom/project_browser-3415079/sveltejs/.eslintrc.js".
If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.

Steps to reproduce

Enable ESLint in .gitlab-ci.yml. Observe failure in pipeline.

Proposed resolution

I think the problem might be due to Svelte components not being installed. Going to try to add yarn commands to before script.

CommentFileSizeAuthor
#3 Screenshot from 2024-02-16 00-39-55.png18.79 KBlostcarpark

Issue fork project_browser-3421699

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

lostcarpark created an issue. See original summary.

lostcarpark opened merge request !435

lostcarpark’s picture

lostcarpark commented
StatusFileSize
new Screenshot from 2024-02-16 00-39-55.png18.79 KB

I haven't managed to get the regular "eslint" check to validate the Svelte code. This could be because it's tailored to validate Drupal style JS.

I've set up a separate "sveltejs-eslint" job to run the existing yarn lint:svelte command from DrupalCI, and it's working correctly.

My thinking is we use the regular "eslint" job to only validate non-Svelte JS. I was looking for a way to exclude the directory. The only way I've found to add rm sveltejs to the before script. This seems a little hacky, but is okay since the whole environment is discarded after the validation job completes.

I don't think we were validating the project_browser.admin.js file previously. We're now finding errors in that, but I think that should be a follow on issue.

Screenshot of validation steps

fjgarlin made their first commit to this issue’s fork.

fjgarlin’s picture

fjgarlin commented
Status: Active » Reviewed & tested by the community

I tried the approach with .eslintignore but it didn't work, so reverted back to what @lostcarpark initially did, which I think it's also ok.

The only really small change I did was to change the name of the job from "svelte-eslint" to "eslint-svelte" so it would appear right next to the "eslint" job in the pipelines (alphabetical order).

Given that that was my only change, I'm marking this RTBC.

PS: unrelated to this issue. If we have GitLab CI in such a good state already, do we need DrupalCI for the project at all?

chrisfromredfin made their first commit to this issue’s fork.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Reviewed & tested by the community » Needs work

Rebased and ESLint is running, but now there's 8 warnings and 1 error that should be fixed, and that's progress! 

$ $CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint --no-error-on-unmatched-pattern --ignore-pattern="*.es6.js" --resolve-plugins-relative-to=$CI_PROJECT_DIR/$_WEB_ROOT/core --ext=.js,.yml . || true
/builds/project/project_browser/web/modules/custom/project_browser/js/project_browser.admin.js
   10:2   warning  Unexpected unnamed function                                  func-names
   23:21  warning  'settings' is defined but never used                         no-unused-vars
   66:19  warning  Unary operator '++' used                                     no-plusplus
   74:40  warning  Unexpected unnamed function                                  func-names
   74:50  warning  'swappedRow' is defined but never used                       no-unused-vars
   80:26  warning  Unexpected unnamed function                                  func-names
  108:22  warning  Unexpected unnamed function                                  func-names
  108:32  warning  'event' is defined but never used                            no-unused-vars
  114:45  error    A constructor name should not start with a lowercase letter  new-cap
✖ 9 problems (1 error, 8 warnings)
lostcarpark’s picture

lostcarpark commented

I have been fixing errors reported by ESLint. Changes are as follows:

  • Anonymous functions have been given names. This is because when a JS error is reported, if there is an anonymous function in the stack, it will just show "anonymous" in the output, and it will be difficult to debug where it occurred.
  • Where the last parameter in a function definition isn't used, it has been removed.
  • ESLint does not like the ++ operator, so code using it has been expanded to make clearer.

There is one reported issue that I don't know how to resolve:

tableDrag.rowObject = new tableDrag.row(row[0]);

is reporting "A constructor name should not start with a lowercase letter". I'm sure what to do about this since the constructor is defined in a library.

lostcarpark’s picture

lostcarpark commented

To fix the last ESLint error, I changed from:

tableDrag.rowObject = new tableDrag.row(row[0]);

to

const Row = tableDrag.row;
tableDrag.rowObject = new Row(row[0]);

This creates a new reference to the tableDrag.row class with a properly capitalized name. Effectively, this tricks ESLint into believing the constructor is correctly capitalized.

I've done some manual testing, and it seems to be working correctly. All tests are passing.

lostcarpark’s picture

lostcarpark commented

After discussing with @chrisfromredfin, changed to the following:

// eslint-disable-next-line new-cap
tableDrag.rowObject = new tableDrag.row(row[0]);
lostcarpark’s picture

lostcarpark commented
Status: Needs work » Needs review
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs review » Needs work

Quick review is wonderful; just need clarity on one question/comment.

lostcarpark’s picture

lostcarpark commented

Please also see @fjgarlin's comment #5 above.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs work » Fixed

Let there be linting!

Status: Fixed » Closed (fixed)

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