Fix ESLint pipeline

I would recommend holding off on all of these pipeline fixes until the D11 branch is merged. Otherwise we're going to have major conflicts which will be very hard to manage.

D11 branch is merged.

Please don't make a merge request until there is at least one commit on the feature branch.

The majority of the fails are in yml files, only a handful are in js files. It looks like eslint is treating yaml as js, and digging a little deeper it's probably because webform has its own .eslintrc.json file that's missing some config that the default pipelines has. Going to try merge some of the defaults from https://git.drupalcode.org/project/gitlab_templates/-/blob/main/assets/i... and see what we get.

I couldn't get it working merging the bits from the default config so I tried just removing the custom config altogether. Turns out the config that core uses is massively different to what webform uses, so it's resulted in a very large patch. Not sure where to go from here, giving up for now.

Are the changes generated by a linter?

The diff is + 3346 − 2771, I certainly didn't do that by hand. The pipeline for 7f4b4891 spat out the patch and I applied it. I would guess that is perhaps too disruptive as it will conflict with existing merge requests, but on the other hand it might be better to bite the bullet and get it done.

That makes sense. I'm not aware of any open MRs that change JS.

The eslint stage still needs work to match Drupal Core's JavaScript code style

📌 Make tests pass Active needs to be resolved before we can move ahead.

Tests are passing again on 6.3.x. Please reroll and get tests to pass.

Tests are passing again on 6.3.x. Please reroll and get tests to pass.

I rebased the merge request. Tests on this merge request are not passing and there are still ESLint issues.

tom konda → made their first commit to this issue’s fork.

I am finding the scope of this MR daunting. I am unsure how we can test these changes and ensure nothing is broken.

With the D11 upgrade, we broke the MRs into smaller tickets.

Theoretically we could make this less daunting by having smaller child issues that each address specific issues. For example, one issue could remove .eslintrc.json, another could remove all the "use strict" statements, and so on. It would create a sizeable list of child issues, and each one would probably impact nearly the same number of files, but each issue would be much easier to review because it would be the same pattern everywhere.

I'd be happy to get started with this if there's agreement on the approach.

I don't mind having it split up. Comment #9 says that these changes came from an automated linter, so unless there is a bug in the linter, they ought to make no changes.

📌 Remove .eslintrc.json Active is merged.

Thanks @liam-morland. I also now have an MR up at 📌 Remove "use strict" statements from JS files Active . I was thinking once that is merged, it would be good to start trying to start chipping away at the no-jquery where direct replacements exist. Open to other suggestions, however.

The current eslint pipeline raises 23 errors like "'use strict' is unnecessary inside of modules", but this merge request removes use strict from 84 files. As I understand it, this should only be removed from the 23 files that are modules.

I think gradually fixing eslint and other JS issue in smaller MRs might be the only approach because there is no JavaScript test coverage in the module.

Am I misunderstanding something about when 'use strict' is supposed to be in a JS file? I thought it was supposed to be there except in module files. Or put another way, it's supposed to be there unless eslint raises an error about it being there.