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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Fix unintended overwrite of eslint no-restricted-syntax #62301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
260c727
Fix unintended overwrite of eslint `no-restricted-syntax`
mirka Jun 4, 2024
f6e705f
Fix errors in components
mirka Jun 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 70 additions & 63 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,72 @@ const restrictedImports = [
},
];

const restrictedSyntax = [
Copy link
Member Author

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.

// NOTE: We can't include the forward slash in our regex or
// we'll get a `SyntaxError` (Invalid regular expression: \ at end of pattern)
// here. That's why we use \\u002F in the regexes below.
{
selector:
'ImportDeclaration[source.value=/^@wordpress\\u002F.+\\u002F/]',
message: 'Path access on WordPress dependencies is not allowed.',
},
{
selector:
'CallExpression[callee.name="deprecated"] Property[key.name="version"][value.value=/' +
majorMinorRegExp +
'/]',
message:
'Deprecated functions must be removed before releasing this version.',
},
{
selector:
'CallExpression[callee.object.name="page"][callee.property.name="waitFor"]',
message:
'This method is deprecated. You should use the more explicit API methods available.',
},
{
selector:
'CallExpression[callee.object.name="page"][callee.property.name="waitForTimeout"]',
message: 'Prefer page.waitForSelector instead.',
},
{
selector: 'JSXAttribute[name.name="id"][value.type="Literal"]',
message:
'Do not use string literals for IDs; use withInstanceId instead.',
},
{
// Discourage the usage of `Math.random()` as it's a code smell
// for UUID generation, for which we already have a higher-order
// component: `withInstanceId`.
selector:
'CallExpression[callee.object.name="Math"][callee.property.name="random"]',
message:
'Do not use Math.random() to generate unique IDs; use withInstanceId instead. (If you’re not generating unique IDs: ignore this message.)',
},
{
selector:
'CallExpression[callee.name="withDispatch"] > :function > BlockStatement > :not(VariableDeclaration,ReturnStatement)',
message:
'withDispatch must return an object with consistent keys. Avoid performing logic in `mapDispatchToProps`.',
},
{
selector:
'LogicalExpression[operator="&&"][left.property.name="length"][right.type="JSXElement"]',
message:
'Avoid truthy checks on length property rendering, as zero length is rendered verbatim.',
},
];

/** `no-restricted-syntax` rules for components. */
const restrictedSyntaxComponents = [
{
selector:
'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="__experimentalIsFocusable"])) JSXAttribute[name.name="disabled"]',
message:
'`disabled` used without the `__experimentalIsFocusable` prop. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or preventing focus from returning to a trigger element. (Ignore this error if you truly mean to disable.)',
},
];

module.exports = {
root: true,
extends: [
Expand Down Expand Up @@ -147,63 +213,7 @@ module.exports = {
disallowTypeAnnotations: false,
},
],
'no-restricted-syntax': [
'error',
// NOTE: We can't include the forward slash in our regex or
// we'll get a `SyntaxError` (Invalid regular expression: \ at end of pattern)
// here. That's why we use \\u002F in the regexes below.
{
selector:
'ImportDeclaration[source.value=/^@wordpress\\u002F.+\\u002F/]',
message:
'Path access on WordPress dependencies is not allowed.',
},
{
selector:
'CallExpression[callee.name="deprecated"] Property[key.name="version"][value.value=/' +
majorMinorRegExp +
'/]',
message:
'Deprecated functions must be removed before releasing this version.',
},
{
selector:
'CallExpression[callee.object.name="page"][callee.property.name="waitFor"]',
message:
'This method is deprecated. You should use the more explicit API methods available.',
},
{
selector:
'CallExpression[callee.object.name="page"][callee.property.name="waitForTimeout"]',
message: 'Prefer page.waitForSelector instead.',
},
{
selector: 'JSXAttribute[name.name="id"][value.type="Literal"]',
message:
'Do not use string literals for IDs; use withInstanceId instead.',
},
{
// Discourage the usage of `Math.random()` as it's a code smell
// for UUID generation, for which we already have a higher-order
// component: `withInstanceId`.
selector:
'CallExpression[callee.object.name="Math"][callee.property.name="random"]',
message:
'Do not use Math.random() to generate unique IDs; use withInstanceId instead. (If you’re not generating unique IDs: ignore this message.)',
},
{
selector:
'CallExpression[callee.name="withDispatch"] > :function > BlockStatement > :not(VariableDeclaration,ReturnStatement)',
message:
'withDispatch must return an object with consistent keys. Avoid performing logic in `mapDispatchToProps`.',
},
{
selector:
'LogicalExpression[operator="&&"][left.property.name="length"][right.type="JSXElement"]',
message:
'Avoid truthy checks on length property rendering, as zero length is rendered verbatim.',
},
],
'no-restricted-syntax': [ 'error', ...restrictedSyntax ],
},
overrides: [
{
Expand Down Expand Up @@ -262,12 +272,8 @@ module.exports = {
rules: {
'no-restricted-syntax': [
'error',
{
selector:
'JSXOpeningElement[name.name="Button"]:not(:has(JSXAttribute[name.name="__experimentalIsFocusable"])) JSXAttribute[name.name="disabled"]',
message:
'`disabled` used without the `__experimentalIsFocusable` prop. Disabling a control without maintaining focusability can cause accessibility issues, by hiding their presence from screen reader users, or preventing focus from returning to a trigger element. (Ignore this error if you truly mean to disable.)',
},
...restrictedSyntax,
...restrictedSyntaxComponents,
],
},
},
Expand Down Expand Up @@ -390,6 +396,7 @@ module.exports = {
rules: {
'no-restricted-syntax': [
'error',
...restrictedSyntax,
Copy link
Member Author

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.

{
selector:
':matches(Literal[value=/--wp-admin-theme-/],TemplateElement[value.cooked=/--wp-admin-theme-/])',
Expand Down
3 changes: 3 additions & 0 deletions packages/components/src/checkbox-control/stories/index.story.tsx
View file
Open in desktop
Copy link
Member Author

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.

Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,16 @@ export const WithCustomLabel: StoryFn< typeof CheckboxControl > = ( {
setChecked( v );
onChange( v );
} }
// Disable reason: For simplicity of the code snippet.
// eslint-disable-next-line no-restricted-syntax
id="my-checkbox-with-custom-label"
aria-describedby="my-custom-description"
/>
<VStack>
<label htmlFor="my-checkbox-with-custom-label">
My custom label
</label>
{ /* eslint-disable-next-line no-restricted-syntax */ }
<div id="my-custom-description" style={ { fontSize: 13 } }>
A custom description.
</div>
Expand Down
Loading