-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Fix unintended overwrite of eslint no-restricted-syntax
#62301
Conversation
@@ -83,6 +83,72 @@ const restrictedImports = [ | |||
}, | |||
]; | |||
|
|||
const restrictedSyntax = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunk of code was just extracted. No changes.
@@ -390,6 +396,7 @@ module.exports = { | |||
rules: { | |||
'no-restricted-syntax': [ | |||
'error', | |||
...restrictedSyntax, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We also need to add ...restrictedSyntaxComponents
here, but since it will catch a bunch of previously uncaught violations, I'll address this in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previously uncaught violation due to the configuration error.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Size Change: 0 B Total Size: 1.74 MB ℹ️ View Unchanged
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
(edit: nvm I got things mixed up, good to go)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍🏻
…62301) * Fix unintended overwrite of eslint `no-restricted-syntax` * Fix errors in components Co-authored-by: mirka <0mirka00@git.wordpress.org> Co-authored-by: DaniGuardiola <daniguardiola@git.wordpress.org> Co-authored-by: fullofcaffeine <fullofcaffeine@git.wordpress.org>
What?
Fixes the
eslintrc
file so that theno-restricted-syntax
rules are applied as intended.Why?
Any settings for the same rule (
no-restricted-syntax
in this case) will be completely overwritten by subsequentoverrides
settings, not merged. I had unconsciously been treating it like they would be merged 🙈How?
Extracted out the restricted syntax rules to be more composable. (This is similar to what had already been done with
restrictedImports
.)Testing Instructions
✅ The "global" restricted syntax rules at the root level should also trigger on the following files, as originally intended: