Update ESLint configuration for json5 files

[alanorth]

References

• Related to src/assets/i18n/en.json5: lint #2306
• Closes yarn merge-i18n script always unquotes the "title" key #2309

Description

Update the ESLint configuration for json5 files.

Instructions for Reviewers

List of changes in this PR:

• Use plugin:jsonc/recommended-with-json5 instead of plugin:jsonc/recommended-with-jsonc
• Use jsonc/no-irregular-whitespace plugin instead of ESLint's no-irregular-whitespace

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes ESLint validation using npm run lint
• My PR doesn't introduce circular dependencies (verified via npm run check-circ-deps)
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new libraries/dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
• If my PR includes new features or configurations, I've provided basic technical documentation in the PR itself.
• If my PR fixes an issue ticket, I've linked them together.

[alanorth]

I will restart the discussion on this after DSpace 7.6 gets released. In that time it would be good to think about the irregular space rule and the one blank line rule so we can start enforcing style in CI lint tests.

[kshepherd]

@alanorth some tests failing but a rebase will probably fix them i imagine. i'll give this a check

[alanorth]

Thanks @kshepherd. I've rebased on latest main and the tests are running now. Let's see...

[tdonohue]

It looks like this JSONC validation behavior was added in #2110 by @alexandrevryghem . So, it'd be good to verify that switching to JSON5 validation should have no side-effects. @alexandrevryghem : your feedback on this PR would be welcome.

@alanorth : I also notice we are still pulling in the eslint-plugin-jsonc plugin in .eslintrc.json here: https://github.com/DSpace/dspace-angular/blob/main/.eslintrc.json#L11 and https://github.com/DSpace/dspace-angular/blob/main/.eslintrc.json#L15 (strangely in two places). Should those be removed in this PR if we are no longer planning to use JSONC?

Finally, in my opinion this must update the current JSON5 files...otherwise the main branch will fail after we merge this. However, you can wait to update those JSON5 files until we agree on the proper rules.

[alanorth]

@alanorth : I also notice we are still pulling in the eslint-plugin-jsonc plugin in .eslintrc.json here: main/.eslintrc.json#L11 and main/.eslintrc.json#L15 (strangely in two places). Should those be removed in this PR if we are no longer planning to use JSONC?

Thanks @tdonohue. Well spotted! We need to keep one, as the eslint-plugin-jsonc plugin is still providing the linting for JSON5. I will remove the other.

Finally, in my opinion this must update the current JSON5 files...otherwise the main branch will fail after we merge this. However, you can wait to update those JSON5 files until we agree on the proper rules.

Sounds good to me. The JSON5 validation in the first commit will fix #2309, while the following three propose a more consistent and slightly stricter style with regards to whitespace:

• Use jsonc/no-irregular-whitespace plugin instead of ESLint's no-irregular-whitespace at the plugin's recommendation
• Use no-multiple-empty-lines plugin to enforce one blank line in between content
• Use comma-spacing plugin to disallow content like this ,

Those are easily and automatically fixable so I think it's worth being consistent.

[kshepherd]

I think the stricter empty lines checks are causing the language files (including those newly updated by i18n-sync) to fail the new check, right? So if we want this PR to pass all the checks we might have to include refactored *.json5 files as part of it, or as a separate PR to merge first?

[alanorth]

Hey @kshepherd. Yes, the tests were failing because of the stricter rules. Perhaps I shouldn't have introduced the comma and whitespace rules, as those are out of the scope.

The main issue I want to solve here is that we are mixing and matching tooling for JSONC and JSON5, which causes the bug in #2309. I will re-work this to focus on that.

[kshepherd]

+1 looks good to me now! thanks @alanorth

[tdonohue]

👍 Thanks @alanorth ! This looks good to me too now. I'm going to also see if we can backport this small change to 7.x/8.x automatically, just for consistency in our ESlint rules.

[dspace-bot]

Backport failed for dspace-7_x, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin dspace-7_x
git worktree add -d .worktree/backport-2317-to-dspace-7_x origin/dspace-7_x
cd .worktree/backport-2317-to-dspace-7_x
git switch --create backport-2317-to-dspace-7_x
git cherry-pick -x 3a057332569cbc7fd5032eb20b0309bd8e66eccd 5e8571de80b071b2e331e217d2d07a4f9c42f425 9896eab6ac096849ac576fdd75802f3b1821ac9d

[dspace-bot]

Successfully created backport PR for dspace-8_x:

• [Port dspace-8_x] Update ESLint configuration for json5 files #4209

[tdonohue]

@alanorth : The backport to 8.x went smoothly, but the backport to 7.x failed (likely just a small code conflict, as the code in 7.x looks similar). If you have time, feel free to create a 7.x version of this PR & I'll get it merged.

[alanorth]

Thanks @tdonohue, I ported it to dspace-7_x manually in #4210.