feat: use @eslint-community dependencies

[MichaelDeBoey]

Closes #15772

CC/ @ota-meshi

[netlify]

✅ Deploy Preview for docs-eslint canceled.

Name	Link
🔨 Latest commit	dd22bf4
🔍 Latest deploy log	https://app.netlify.com/sites/docs-eslint/deploys/6401dd164d569d0008195fac

[aladdin-add]

LGTM.

[mdjermanovic]

Closes #15772

Can we add a test to confirm that this closes #15772?

[mdjermanovic]

ESlint v8 supports Node v17, but @eslint-community/regexpp doesn't:

npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@eslint-community/regexpp@4.2.0',
npm WARN EBADENGINE   required: { node: '^12.0.0 || ^14.0.0 || ^16.0.0 || >=18.0.0' },
npm WARN EBADENGINE   current: { node: 'v17.9.1', npm: '8.11.0' }
npm WARN EBADENGINE }

Can we update @eslint-community/regexpp to support engines that ESLint supports to avoid these warnings?

[ota-meshi]

Can we add a test to confirm that this closes #15772?

I think we can confirm it by adding a test case that checks that the following code is valid to the test for the no-invalid-regexp rule.

new RegExp("\\p{Script=Cpmn}", "u");
new RegExp("\\p{Script=Cypro_Minoan}", "u");
new RegExp("\\p{Script=Old_Uyghur}", "u");
new RegExp("\\p{Script=Ougr}", "u");
new RegExp("\\p{Script=Tangsa}", "u");
new RegExp("\\p{Script=Tnsa}", "u");
new RegExp("\\p{Script=Toto}", "u");
new RegExp("\\p{Script=Vith}", "u");
new RegExp("\\p{Script=Vithkuqi}", "u");

[mdjermanovic]

Can we add a test to confirm that this closes #15772?

I think we can confirm it by adding a test case that checks that the following code is valid to the test for the no-invalid-regexp rule.

new RegExp("\\p{Script=Cpmn}", "u");
new RegExp("\\p{Script=Cypro_Minoan}", "u");
new RegExp("\\p{Script=Old_Uyghur}", "u");
new RegExp("\\p{Script=Ougr}", "u");
new RegExp("\\p{Script=Tangsa}", "u");
new RegExp("\\p{Script=Tnsa}", "u");
new RegExp("\\p{Script=Toto}", "u");
new RegExp("\\p{Script=Vith}", "u");
new RegExp("\\p{Script=Vithkuqi}", "u");

Sounds good to me!

[MichaelDeBoey]

@mdjermanovic v17 are passing, but v17 is EOL so I'm not sure if we really should add explicit support for it tbh 🤔

@aladdin-add @ota-meshi @voxpelli what do you think?

[voxpelli]

@MichaelDeBoey As you know, I prefer us to show the lowest versions we need rather than enforce/include the Node.js EOL in our engines range as that’s doomed to result in needless problems like this

[aladdin-add]

node v12 is EOL, but it's supported. so I suggest it can support node v17 too.

you can drop them in next major version.

[nzakas]

I think this should be either "feat" or "fix", as it is changing functionality (to allow new syntax) rather than "chore". Thoughts?

[mdjermanovic]

I think this should be either "feat" or "fix", as it is changing functionality (to allow new syntax) rather than "chore". Thoughts?

I also think this should be "feat".

[mdjermanovic]

Looks like there's a consensus to update regexpp's engines to cover eslint's engines, so I'm adding a change request to prevent merging until that is done.

[kecrily]

LGTM

[mdjermanovic]

These changes look good. I only have concerns about @eslint-community/eslint-utils having eslint in peerDependencies, as that creates a kind of circular dependency, and when we release ESLint v9 it won't be in the "^6.0.0 || ^7.0.0 || ^8.0.0" range. Would it make sense to change this to "eslint": ">=6" as it used to be before eslint-community/eslint-utils@0bd0441?

[bmish]

These changes look good. I only have concerns about @eslint-community/eslint-utils having eslint in peerDependencies, as that creates a kind of circular dependency, and when we release ESLint v9 it won't be in the "^6.0.0 || ^7.0.0 || ^8.0.0" range. Would it make sense to change this to "eslint": ">=6" as it used to be before eslint-community/eslint-utils@0bd0441?

I'm also in favor of using >= in peer dependencies. I find it to be a better practice to include support for future versions by default, without necessitating an additional release to bump the range.

[nzakas]

It looks like we lost track of this. What are the next steps here?

[MichaelDeBoey]

@nzakas If I'm not mistaken, this PR can be merged as-is, as all remarks are resolved now

[mdjermanovic]

I think the only open question is #16784 (comment).

[nzakas]

@MichaelDeBoey please see @mdjermanovic's previous comment and let us know how you're going to proceed.

[github-actions]

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

[nzakas]

@MichaelDeBoey friendly ping.

[github-actions]

Hi everyone, it looks like we lost track of this pull request. Please review and see what the next steps are. This pull request will auto-close in 7 days without an update.

[MichaelDeBoey]

@nzakas This PR should be ready to be merged

[mdjermanovic]

LGTM, thanks!