Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop support for ESLint < v8.44 #558

Merged
merged 4 commits into from
Aug 26, 2023
Merged

Drop support for ESLint < v8.44 #558

merged 4 commits into from
Aug 26, 2023

Conversation

ota-meshi
Copy link
Owner

@ota-meshi ota-meshi commented Aug 15, 2023

I noticed that ESLint's standard parser can't parse the v flag until ESLint is updated to v8.44.
This PR drops support for ESLint < v8.44.

https://eslint.org/blog/2023/06/eslint-v8.44.0-released/

We could leave the old ESLint support and check the ESLint version, and branch whether or not to test for the v flag, but I think it would be expensive to do that. So I think it makes more sense to drop the old ESLint support.

@changeset-bot
Copy link

changeset-bot bot commented Aug 15, 2023

🦋 Changeset detected

Latest commit: effd266

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
eslint-plugin-regexp Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@RunDevelopment
Copy link
Collaborator

Why exactly would we need to check the ESLint version and branch?

The ESLint parser AST represents regexes as { pattern: string; flags: string }, so why does it matter to us whether the parser supports the v flag? The only issue I see is that rules that add the v flag to regexes (probably a use-v-flag rule) will create code that ESLint can't parse. But that's not our problem. We produce valid JS/TS, and users can turn off the rule if it causes issues.

@ota-meshi
Copy link
Owner Author

The old parser will throw a parsing error with the v flag as an unknown flag. Parsing the v flag requires espree v9.6.0 and is definitely included in eslint v8.44.0 and later.

https://github.com/eslint/espree/releases/tag/v9.6.0

That said, we can certainly use new RegExp('pattern', 'v') if we just want to test our rule.

@RunDevelopment
Copy link
Collaborator

That said, we can certainly use new RegExp('pattern', 'v') if we just want to test our rule.

Oh, so we can't even properly test our v-flag rules right now? That certainly is an issue, and I don't like the idea of having to use RegExp('pattern', 'v') everywhere. We still won't be able to use regex literals in test, because NodeJS 18 doesn't support the v flag, but String.raw`/pattern/v` is better than String.raw`new RegExp('pattern', 'v')`.

@ota-meshi ota-meshi merged commit e591342 into master Aug 26, 2023
5 checks passed
@ota-meshi ota-meshi deleted the update-eslint branch August 26, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants