Problem/Motivation

Hi,

I am enabling Gitlab CI on UI Suite Bootstrap theme, see #3417516: Enable Gitlab CI + Update README + Coding standards.

And I would like to be able to ignore some compiled CSS files in the stylelint job.

Proposed resolution

I have been to override the stylelint job scripts to use the "--ignore-path" option to be able to point to a .stylelintignore file but it does not seem to have an effect. While I already use this option in Makefile and GItlab CI on custom projects, scripts can be found in https://gitlab.com/florenttorregrosa-drupal/docker-drupal-project/-/tree... if needed.

See the results in the MR https://git.drupalcode.org/project/ui_suite_bootstrap/-/merge_requests/163.

Remaining tasks

- Find a "fix"
- Discuss how to add this feature by default in Gitlab CI templates.

Thanks for any help.

CommentFileSizeAuthor
#11 Capture d’écran du 2024-01-30 11-47-26.png77.89 KBGrimreaper
#11 Capture d’écran du 2024-01-30 11-47-32.png69.46 KBGrimreaper

Issue fork gitlab_templates-3417807

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Grimreaper created an issue. See original summary.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

Would adding a $STYLELINT_EXTRA variable help with this?

We've done something similar with $COMPOSER_EXTRA, $_PARALLEL_LINT_EXTRA or $PHPUNIT_EXTRA to allow passing extra parameters to the commands.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

I think this will be the solution, but before going on this, I would like to ensure that the option will work :)

And currently, it is not working on the MR on the related issue.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

It might be related to the path where the file is. I can maybe check it tomorrow.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Assigned: Unassigned » Grimreaper

I think I found the diff in stylelint execution that can explain why in my stack the ignore works.

Drupal.org Gitlab CI:

yarn --silent --cwd $_WEB_ROOT/core stylelint ...

My Makefile:

${APP_PATH}/core/node_modules/.bin/stylelint ...

My Giltab CI:

  script:
    - ./app/core/node_modules/.bin/stylelint
      ...

In my case I use the bin stylelint directly why on drupal.org it is launched with Yarn.

I will try to launch with Yarn locally too to see if the diff comes from that.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

And it is a win! The different results comes from that way to launch stylelint.

Searching how to make it work.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Assigned: Grimreaper » Unassigned

If I place the following line in core/.stylelintrc.json in ignoreFiles, I see that the CSS files are ignored:

"../themes/custom/ui_suite_bootstrap/starterkits/ui_suite_bootstrap_subtheme_example/assets/css/**/*.css"

But in the .stylelintignore, not working. I think the evaluation of the ignore regexes or the path relative to cwd has a bug when used with yarn...

I can't figure out why or where the diff is coming from.

Next step would be to try to put console.log in stylelint codebase but if someone more experienced with stylelint could give feedback, it would be very appreciated.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

https://stylelint.io/user-guide/ignore-code/#files-entirely

Stylelint looks for a .stylelintignore file in process.cwd(). You can also specify a path to your ignore patterns file (absolute or relative to process.cwd()) using the --ignore-path (in the CLI) and ignorePath (in JS) options.

So maybe you need to specify --ignore-pathbecause CWD is changed to ./core in the yarn command.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

Recommended to put the full path like this: $CI_PROJECT_DIR/$_WEB_ROOT/...

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

Yes, the --ignore-path is already what I tried ^^

But its content is resolved differently how stylelint is triggered as mentioned in comment 5 :)

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
FileSize
Capture d’écran du 2024-01-30 11-47-32.png69.46 KB
Capture d’écran du 2024-01-30 11-47-26.png77.89 KB

After some diving into stylelint codebase, and console.log, after console.log debug.

I think I found the "culprit".

app/core/node_modules/stylelint/lib/utils/filterFilePaths.js

// Paths which starts with `..` are not valid for `ignore`, e. g. `../style.css`

So it is not possible to:
- use the --ignore-path option with a .stylelintignore file (relative or absolute path to this file change nothing)
- use the --ignore-pattern option
when it is for linting a file outside the directory where cwd is, those options will be ignored...

With those options (when scanning files inside the directory of cwd), the list of detected files is filtered out before linting:

16 sources checked
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/field.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/indented.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/placeholder.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/tabledrag.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/example/carousel.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/form-required.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/form-text.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/off-canvas.button.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/off-canvas.form.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/user/password.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_block.admin.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_block.editor.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_inline.admin.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_inline.editor.css
 /project/app/themes/custom/ui_suite_bootstrap/templates/patterns/alert/styles/alert.css
 /project/app/modules/custom/ui_skins/tests/themes/ui_skins_test_themes/assets/css/test.css

While when adding ignore into app/core/.stylelintrc.json:

...
  "ignoreFiles": [
    "assets/vendor/**/*.css",
    "tests/Drupal/Tests/Core/Asset/css_test_files/**/*.css",
    "themes/stable9/css/core/assets/vendor/**/*.css",
    "../themes/custom/ui_suite_bootstrap/starterkits/ui_suite_bootstrap_subtheme_example/assets/css/ckeditor5.css"
  ]
}

Then list of detected files is not filtered out but in the results it is mentioned which files had been ignored:

17 of 18 sources checked
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/field.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/indented.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/placeholder.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/component/tabledrag.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/example/carousel.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/form-required.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/form-text.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/off-canvas.button.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/form/off-canvas.form.css
 /project/app/themes/custom/ui_suite_bootstrap/assets/css/user/password.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_block.admin.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_block.editor.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_inline.admin.css
 /project/app/modules/custom/ui_styles/modules/ui_styles_ckeditor5/assets/css/ui_styles_ckeditor5_inline.editor.css
 /project/app/themes/custom/ui_suite_bootstrap/starterkits/ui_suite_bootstrap_subtheme_example/assets/css/ckeditor5.css (ignored)
 /project/app/themes/custom/ui_suite_bootstrap/templates/patterns/alert/styles/alert.css
 /project/app/modules/custom/ui_skins/tests/themes/ui_skins_test_themes/assets/css/test.css
 /project/app/themes/custom/ui_suite_bootstrap/starterkits/ui_suite_bootstrap_subtheme_example/assets/css/form/form-required.css

Attaching screenshots too, with color maybe more readable.

So not possible to handle an ignore that way if launching stylelint with yarn and cwd in core folder.

Maybe we can "play" with the path "../modules/custom/**/*.css" to make it variable. or need to launch stylelint differently or from somewhere else...

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

This other broader issue (#3386841: Reconcile gitlab lint jobs and commit-code-check.sh) is changing the way the lints will be called. Maybe we can take/learn something from there.
MR: https://git.drupalcode.org/project/drupal/-/merge_requests/5498/diffs

--

Or another thought I just had. Maybe we can copy the module's . stylelintignore file, if present, to the "./core" folder.
I'm not sure if that will work though, it's just a thought.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Assigned: Unassigned » Grimreaper

Thanks for the other issue link.

But looking at the MR:

      cd "$TOP_LEVEL/core"
      node_modules/.bin/stylelint --allow-empty-input "$TOP_LEVEL/$FILE"

There will be the same problem because stylelint is executed from the core folder and not from the directory just above.

--

Maybe we can copy the module's . stylelintignore file, if present, to the "./core" folder.

The problem is not that stylelint can't find the .stylelintignore file.

The problem is that either with --ignore-path or --ignore-pattern options, a CSS file outside of the folder from which stylelint is executed (so a CSS file with a relative path starting with "..") can't be ignored.

Maybe it is something to change upstream in stylelint.

Or next stuff I will try is to check if there is an option with stylelint like the --resolve-plugins-relative-to option of eslint. That way let's launch stylelint from the app root folder and make it load core config.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

Regarding the eslint job, it is executed with CI_PROJECT_DIR/$_WEB_ROOT/core/node_modules/.bin/eslint, not with Yarn.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

\o/ https://git.drupalcode.org/project/ui_suite_bootstrap/-/jobs/716123

Proposing a MR for Gitlab template.

Grimreaper opened merge request !116

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Status: Active » Needs work

Re-checking one last stuff on the MR on my theme. To ensure last changes are ok and that I fixed back the report in Gitlab UI.

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Assigned: Grimreaper » Unassigned
Status: Needs work » Needs review

Ok for me.

Thanks for the review.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented

Great stuff! I'll review this tomorrow unless somebody beats me to it.

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Needs review » Needs work

@Grimreaper - can you link two tests runs, one where the ". stylelintignore" file exists in the module and another one where it doesn't?

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented
Status: Needs work » Needs review

Here it is.

Stylelint job from 5.0.x branch: https://git.drupalcode.org/project/ui_suite_bootstrap/-/jobs/716225

Stylelint job from MR https://git.drupalcode.org/project/ui_suite_bootstrap/-/merge_requests/164 where the .stylelintignore file had been deleted: https://git.drupalcode.org/project/ui_suite_bootstrap/-/jobs/724367

fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Needs review » Reviewed & tested by the community

Awesome. Thanks so much. I've reviewed the code and all the feedback was addressed.
The testing is good as well.

RTBC.

  • fjgarlin committed 26400aba on main authored by Grimreaper 
    Issue #3417807 by Grimreaper, fjgarlin: Being able to ignore CSS files
fjgarlin’s picture

fjgarlin CreditAttribution: fjgarlin as a volunteer and at Drupal Association commented
Status: Reviewed & tested by the community » Fixed

Merged and marked this issue as fixed. Thanks!

Grimreaper’s picture

Grimreaper
Primary language French
Location France 🇫🇷
CreditAttribution: Grimreaper at Smile commented

Thanks a lot!

Status: Fixed » Closed (fixed)

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