stylelint task should create default prettier config

MR created that adds the same steps to copy the prettier config as exists on the eslint script.

Hi tomtech,
Could you link to a failing stylelint job please?

I didn't know that styelint could also use the prettierrc.json and prettier.ignore files. But if that is the standard way to do it, then yes we should copy the core ones if the project does not have their own.

Setting to NW as I left a comment in the MR.

@jonathan1055,

Here is a link to a failed gitlab stylelint job.

https://git.drupalcode.org/project/commerce_paypal/-/jobs/6229973

I was able to fix it in the project by copying prettierrc.json from core/.

I copied the lines directly from the eslint job for consistency, but understandably, the line for .prettierignore can be removed. (We can't copy the one from core, as the paths wouldn't be correct.) I'll update the MR.

Thanks for the link to your failing test.

For reference the failing file is /css/paypal-fastlane.css

The core/.prettierrc.json file contains printWidth: 80. To test this MR and to provide ongoing test coverage, I will modify a css file in our Gitlab Templates Downstream → testing project.

I have replicated the problem in GTD. Oddly, the error does not display the full line, like in your job
It just shows that it wants a newline at 6:64 (which is correct) but the full text of the line is not shown.

The suggested newline is not correct.

I think you are overlooking a block in core/.prettierrc.json.

While printWidth is initially set to 80...a few lines down it is specifically overwritten for css files to be 10000, along with using a specific css parser.

See: https://git.drupalcode.org/project/drupal/-/blob/11.x/core/.prettierrc.j...

Specifically, this override for css was added when stylelint configuration added a prettier configuration, along with using a specific css parser. See the commit here:

https://git.drupalcode.org/project/drupal/-/commit/1fbf9f5d2b589e17b7c67...

It's all fine. I editted my comments above, you probably saw the original in e-mail, sorry to add confusion.

I also noticed that the stylelint command has the hard-coded --config $CI_PROJECT_DIR/$_WEB_ROOT/core/.stylelintrc.json which means we are always using the core .stylelintrc.json. Do you think many/any projects have their own .stylelintrc.json ? Maybe we should change that, and do a link to the core link only if there no .stylelintrc.json in the project?

As far as this issue and this MR goes, things look good. Marking it RTBC unless you want to address what was mentioned in #10. That could be done here or in a follow-up issue if needed. Let me know if you want this to be merged as is.

Forgot to actually mark it RTBC.

Merged. Thanks!

Thanks for merging. Can we leave this issue open, as I think we should address #10. I need to check exactly how stylelint detects it's config file. If the first place it looks is in the current directory then we can simply do "if the project has no .stylelintrc.json in project root then create a link to the core file". Should be easy to test.

Cool. Back to "needs work" to try to address #10. The merged change was easy enough to go into "main".

According to the stylelint documentation there are multiple acceptable names for the config file, of which .stylelintrc.json is just one. So our job needs to check for any file starting stylelint.config or .stylelintrc and if none found then we link to the core file. That covers them all apart from if the config is specified within the project's package.json file. I doubt if many projects use that, and if they did then it is already not working for them in Gitlab Templates because we hard-code the core file. So this proposed change does not make that situation any worse.

First attempt failed, we can't link or copy the core file into the project folder because it contains paths that then become invalid:

Error: Could not find "stylelint-config-standard". Do you need to install the package or use the "configBasedir" option?
    at configurationError (file:///builds/project/gitlab_templates_downstream/web/core/node_modules/stylelint/lib/utils/configurationError.mjs:12:49)

So the alternative is to dynamically add the --config when it is needed.

MR399 is ready for initial review. The downstream jobs pass with no changes. The core file is still being used.

I have also tested it in GTD branch 3503337-mr-testing-d10-plus
Here is the starting point, from d10-plus with the added (very simple) stylelint.config.js config file in the project root
The job intentionally fails due to not allowing color names. You can see STYLELINT_CONFIG= is blank in the log because the project has it's own stylelint.config.js

Then modify the local file to use color names and the job passes. This demonstrates that the local config (if is exists) is active and controls the result of the job.

There are two lines of debug to remove, but this is ready for review

The code looks good. I just made a minor suggestion on the debug code.

Once that is addressed and the debug code is removed, I can RTBC.

Thanks for the review, I applied your suggestion.
Here's the downstream d9-basic job and the d10-plus job. All OK.

However I tested again on the 3503337-mr-testing-d10-plus branch which have a local config file.

echo "Detect local configuration files:"
ls -la .stylelint* stylelint* || echo "None found."

The None found. is still printed, even though one file is found. This is because the other criteria is still false.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/j...

In a local terminal ls -la (.stylelint|stylelint)* || echo "None found." using a single regex works as intended, and we do not get the echo if one file is found. so I will try this in the pipeline.

This works fine locally, but in the pipeline we get

Detect local configuration files:
$ ls -la (.stylelint|stylelint)* || echo "None found."
/scripts-173036-6320433/step_script: eval: line 360: syntax error near unexpected token `('

https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/j...
It's a pain that my local checking does not match the pipeline. But I'm not running gitlab temapltes pipelines locally, I was just testing the command in a terminal running zsh. Obviously there are variations. Any ideas what the pipeline job will accept as the equivalent?

It might be due to the symlinks. What if we run it in the source folder instead of the symlinked one?

Also, maybe in order to use regex with ls, we might need to chain the command with a grep or similar (eg: https://stackoverflow.com/questions/15345936/regular-expression-usage-wi...)

I don't think it is the symlinks, because the error is with the syntax of the command, not the result.

Locally ls -la .stylelint* || ls -la stylelint* || echo "None found." works as intended. We only get the echo if neither of the others are found. It has the downside that if the first mnatches then we don't get a listing for the second one at all. But I will try this in the pipeline. Yes we could also pipe to a grep. The dificulty lies with the ls having to include files that both do and not start with a dot. A wildcard at the start does not return files starting with .

Tested

ls -la .stylelint* || ls -la stylelint* || echo "None found."</code
OK in GTD when no files found https://git.drupalcode.org/project/gitlab_templates_downstream/-/jobs/6321204#L94
But when one file is found, it works, but we still get  missleading message
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/jobs/6321287#L47

The grep solution is better, as it shows all files we are interested in. The pattern does not use < code>^

for "starts with" because the filename is in the middle of the output. The space has this function instead, to avoid files with 'stylelint' in the middle of the name.
ls -la | grep -E ' \.?stylelint' || echo 'none found'

When there are no files it works correctly
When files exist, it works correctly too

Just thinking that the Styelint docs page could be updated with this, but the code is ready for review. I will add the doc update, hence still NW

The solution looks good. RTBC pending the documentation changes.

Merged. Thanks for the new additions.

Ah, you said "RTBC pending the documentation changes" and I had not made those changes yet. I thought that was a bit of a gamble to set RTBC early.

No worries, to save a further MR here I will make the changes on MR328 on 📌 Documentation pages Active . There are some other things already in there, so will be good to get that merged soon.

Oh dear. My bad, you are right, I kind of put a trap for myself. I was combing through RTBC issues, I reviewed the code, which already looked good, but didn't see my own comment 🤦.

Sorry about this. Happy to have an extra MR here or just add to the documentation issue.

Absolutely no problem and no apology needed. I could have also set this back to NW to save you falling into your own trap ;-)

I will do the work on MR328, as that needs to be merged anyway, no need for another MR here.

I was trying out something else in a local terminal and got an error saying test: too many arguments which was caused by the new line

STYLELINT_CONFIG=$(test -e .stylelintrc* -o -e stylelint.config.*  && echo "" || echo "--config $CI_PROJECT_DIR/$_WEB_ROOT/core/.stylelintrc.json")

After further investigation I realised that test is only expecting one file, and if the wildcard causes two or more files to be returned we get this error. So I think it might be better to replace the above with something like

ls -a | grep -E -q '\.stylelintrc*|stylelint\.config\.*' && STYLELINT_CONFIG="" || STYLELINT_CONFIG="--config $CI_PROJECT_DIR/$_WEB_ROOT/core/.stylelintrc.json"

Locally this copes with multiple matching files. I know this is an edge case, but it would be nice to avoid the error, given that we have just introduced this new part of the job.

Here's an example of the error
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/j...
But in this situation the outcome is OK, because the blank value of STYLELINT_CONFIG is actually what is wanted here. So maybe we don't need to change anything, as it does give the correct result.

Good catch! Can you create an MR for the fix? Otherwise I can do it too. No need for it to be today as we’re both probably ending the day.

I've created MR401 here
Downstream jobs are fine.

Tested on the same GTD testing branch as above and now we don't get the error and we do get the local config being used.
https://git.drupalcode.org/issue/gitlab_templates_downstream-3503337/-/p...
The job fails, which is expected because the local config has different rules.

Ready for review. I was going to add the missing help here, but actually I will do that on 📌 Documentation pages Active so thet we get those other help fixes in too.

The changes look and the tests show that both cases (files present vs not) are working. RTBC and will merge shortly.

Merged. Thanks for catching this!

Do you think we should make an announcement on Slack that styleleint jobs are now using the core prettier config when they were not doing so before. Quite a few project pipelines are retruning new warnings in Stylelint where previously nothing was being checked, or it was using less rigorous set of rules.

Also, in writing the documentation update for Stylelint, I had the thought that there could be alternative file names for the prettier config file, just like there is for the stylelint file, and there are lots of formats. So I will make a quick MR here to cater for them, just for completeness.

Tested OK in downstream d10-plus and d9-basic which do not have any prettier config files.

In the 3503337-mr-testing-d10-plus branch, the log shows that it uses the local stylelint and prettier config files.

MR402 is ready for review

I merged the new MR to cater for more naming patterns. Thanks!

I'd say that this is not being too disruptive so far, so we can just help when/as needed via slack. If the volume gets higher we can post something about it.