Skip to content

Commit

Permalink
ESLint rule to detect forwardRef components without display name (#3451)
Browse files Browse the repository at this point in the history
* Added es lint rule to ensure display name for forwardRef components

* Added display name for `forwardRef` components

* removed fix

* Added cl

* Updated message in tests

* Check if nodes parent is a member expression

* Updated cl

* Fixed issue with rule

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
  • Loading branch information
ashikmeerankutty and chandlerprall committed May 14, 2020
1 parent e8eb712 commit 574429d
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 0 deletions.
1 change: 1 addition & 0 deletions .eslintplugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ exports.rules = {
i18n: require('./scripts/eslint-plugin/i18n'),
'href-with-rel': require('./scripts/eslint-plugin/rel'),
'require-license-header': require('./scripts/eslint-plugin/require_license_header'),
'forward-ref': require('./scripts/eslint-plugin/forward_ref_display_name'),
};
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ module.exports = {
"prefer-template": "error",
"local/i18n": "error",
"local/href-with-rel": "error",
"local/forward-ref": "error",
"local/require-license-header": [
'warn',
{
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## [`master`](https://github.com/elastic/eui/tree/master)

- Added exports for `EuiSteps` and related components types ([#3471](https://github.com/elastic/eui/pull/3471))
- Added `displayName` to components using `React.forwardRef` ([#3451](https://github.com/elastic/eui/pull/3451))

## [`24.0.0`](https://github.com/elastic/eui/tree/v24.0.0)

Expand Down
77 changes: 77 additions & 0 deletions scripts/eslint-plugin/forward_ref_display_name.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'Enforce display name to forwardRef components',
},
},
create: function(context) {
const forwardRefUsages = [];
const displayNameUsages = [];
return {
VariableDeclarator(node) {
if (node.init && node.init.type === 'CallExpression') {
if (
node.init.callee &&
node.init.callee.type === 'MemberExpression'
) {
if (
node.init.callee.property &&
node.init.callee.property.name === 'forwardRef'
) {
forwardRefUsages.push(node.id);
}
}
if (node.init.callee && node.init.callee.name === 'forwardRef') {
forwardRefUsages.push(node.id);
}
}
},
MemberExpression(node) {
const { property } = node;
if (
property &&
property.type === 'Identifier' &&
property.name === 'displayName'
) {
displayNameUsages.push(node.object);
}
},
'Program:exit'() {
forwardRefUsages.forEach(identifier => {
if (!isDisplayNameUsed(identifier)) {
context.report({
node: identifier,
message: 'Forward ref components must use a display name',
});
}
});
},
};
function isDisplayNameUsed(identifier) {
const node = displayNameUsages.find(
displayName => displayName.name === identifier.name
);
return !!node;
}
},
};
47 changes: 47 additions & 0 deletions scripts/eslint-plugin/forward_ref_display_name.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import rule from './forward_ref_display_name.js';
const RuleTester = require('eslint').RuleTester;

const ruleTester = new RuleTester({
parser: 'babel-eslint',
});

const valid = [
`const Component = React.forwardRef<ref>(() => {})
Component.displayName = "EuiBadgeGroup"
`,
];

const invalid = [
{
code: 'const Component = React.forwardRef<ref>(() => {})',
errors: [
{
message: 'Forward ref components must use a display name',
},
],
},
];

ruleTester.run('forward_ref_display_name', rule, {
valid,
invalid,
});
2 changes: 2 additions & 0 deletions src/components/color_picker/color_picker_swatch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,5 @@ export const EuiColorPickerSwatch = forwardRef<
/>
);
});

EuiColorPickerSwatch.displayName = 'EuiColorPickerSwatch';
2 changes: 2 additions & 0 deletions src/components/color_picker/saturation.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -208,3 +208,5 @@ export const EuiSaturation = forwardRef<HTMLDivElement, EuiSaturationProps>(
);
}
);

EuiSaturation.displayName = 'EuiSaturation';
2 changes: 2 additions & 0 deletions src/components/datagrid/data_grid_header_row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,6 @@ const EuiDataGridHeaderRow = forwardRef<
);
});

EuiDataGridHeaderRow.displayName = 'EuiDataGridHeaderRow';

export { EuiDataGridHeaderRow };
2 changes: 2 additions & 0 deletions src/components/form/range/range_slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,3 +94,5 @@ export const EuiRangeSlider = forwardRef<HTMLInputElement, EuiRangeSliderProps>(
);
}
);

EuiRangeSlider.displayName = 'EuiRangeSlider';
2 changes: 2 additions & 0 deletions src/components/form/range/range_wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,5 @@ export const EuiRangeWrapper = forwardRef<HTMLDivElement, EuiRangeWrapperProps>(
);
}
);

EuiRangeWrapper.displayName = 'EuiRangeWrapper';

0 comments on commit 574429d

Please sign in to comment.