Skip to content

Commit

Permalink
Lint rule for requiring ts-ignore descriptions (#3584)
Browse files Browse the repository at this point in the history
* upgrades @typescript-eslint plugin and parser (removes deprecated rules)

as well as runs prettier on .eslintrc.js

* turns off new (unwanted) rules

* only allows ts-ignore and ts-expect-error with descriptions

* tweaks for new @typescript-eslint version

* clears error for 'prefer-as-const' lint rule

this used to be required to get a behavior that has since been filpped to always work the 'as const' way, so this is no longer necessary (hence the lint rule complaining)

* removes disable for rule that no longer exists

interface-name-prefix was replaced by naming-convention in @typescript-eslint v3

* removes ts-ignore-next-line usages (that's not a thing)

* use  for sort Direction

* uses  for all ts-ignores that need it

* solves simplest ts-ignore cases

some simply just needed to be removed (mostly due to ignores for files that didn't used to have types but now do)

* Clean up ts-ignore todos

Co-authored-by: Chandler Prall <chandler.prall@gmail.com>
  • Loading branch information
dimitropoulos and chandlerprall committed Jun 11, 2020
1 parent 501f0c2 commit e0f79ff
Show file tree
Hide file tree
Showing 30 changed files with 162 additions and 146 deletions.
112 changes: 61 additions & 51 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,66 +50,76 @@ module.exports = {
],
plugins: ['jsx-a11y', 'prettier', 'local', 'react-hooks'],
rules: {
"prefer-template": "error",
"local/i18n": "error",
"local/href-with-rel": "error",
"local/forward-ref": "error",
"local/require-license-header": [
'prefer-template': 'error',
'local/i18n': 'error',
'local/href-with-rel': 'error',
'local/forward-ref': 'error',
'local/require-license-header': [
'warn',
{
license: APACHE_2_0_LICENSE_HEADER,
},
],
"no-use-before-define": "off",
"quotes": ["warn", "single", "avoid-escape"],
"jsx-a11y/accessible-emoji": "error",
"jsx-a11y/alt-text": "error",
"jsx-a11y/anchor-has-content": "error",
"jsx-a11y/aria-activedescendant-has-tabindex": "error",
"jsx-a11y/aria-props": "error",
"jsx-a11y/aria-proptypes": "error",
"jsx-a11y/aria-role": "error",
"jsx-a11y/aria-unsupported-elements": "error",
"jsx-a11y/heading-has-content": "error",
"jsx-a11y/html-has-lang": "error",
"jsx-a11y/iframe-has-title": "error",
"jsx-a11y/interactive-supports-focus": "error",
"jsx-a11y/media-has-caption": "error",
"jsx-a11y/mouse-events-have-key-events": "error",
"jsx-a11y/no-access-key": "error",
"jsx-a11y/no-distracting-elements": "error",
"jsx-a11y/no-interactive-element-to-noninteractive-role": "error",
"jsx-a11y/no-noninteractive-element-interactions": "error",
"jsx-a11y/no-noninteractive-element-to-interactive-role": "error",
"jsx-a11y/no-redundant-roles": "error",
"jsx-a11y/role-has-required-aria-props": "error",
"jsx-a11y/role-supports-aria-props": "error",
"jsx-a11y/scope": "error",
"jsx-a11y/tabindex-no-positive": "error",
"jsx-a11y/label-has-associated-control": "error",
'no-use-before-define': 'off',
quotes: ['warn', 'single', 'avoid-escape'],
camelcase: 'off',
'jsx-a11y/accessible-emoji': 'error',
'jsx-a11y/alt-text': 'error',
'jsx-a11y/anchor-has-content': 'error',
'jsx-a11y/aria-activedescendant-has-tabindex': 'error',
'jsx-a11y/aria-props': 'error',
'jsx-a11y/aria-proptypes': 'error',
'jsx-a11y/aria-role': 'error',
'jsx-a11y/aria-unsupported-elements': 'error',
'jsx-a11y/heading-has-content': 'error',
'jsx-a11y/html-has-lang': 'error',
'jsx-a11y/iframe-has-title': 'error',
'jsx-a11y/interactive-supports-focus': 'error',
'jsx-a11y/media-has-caption': 'error',
'jsx-a11y/mouse-events-have-key-events': 'error',
'jsx-a11y/no-access-key': 'error',
'jsx-a11y/no-distracting-elements': 'error',
'jsx-a11y/no-interactive-element-to-noninteractive-role': 'error',
'jsx-a11y/no-noninteractive-element-interactions': 'error',
'jsx-a11y/no-noninteractive-element-to-interactive-role': 'error',
'jsx-a11y/no-redundant-roles': 'error',
'jsx-a11y/role-has-required-aria-props': 'error',
'jsx-a11y/role-supports-aria-props': 'error',
'jsx-a11y/scope': 'error',
'jsx-a11y/tabindex-no-positive': 'error',
'jsx-a11y/label-has-associated-control': 'error',

"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn",
'react-hooks/rules-of-hooks': 'error',
'react-hooks/exhaustive-deps': 'warn',

"@typescript-eslint/array-type": ["error", { "default": "array-simple" }],
// Nice idea...but not today.
"@typescript-eslint/ban-ts-ignore": "off",
"@typescript-eslint/camelcase": "off",
"@typescript-eslint/class-name-casing": "off",
"@typescript-eslint/explicit-function-return-type": "off",
"@typescript-eslint/explicit-member-accessibility": "off",
"@typescript-eslint/indent": "off",
"@typescript-eslint/no-empty-interface": "off",
"@typescript-eslint/no-explicit-any": "off",
"@typescript-eslint/no-non-null-assertion": "off",
"@typescript-eslint/no-parameter-properties": "off",
"@typescript-eslint/no-triple-slash-reference": "off",
"@typescript-eslint/no-unused-vars": ["error", { "argsIgnorePattern": "^_", "ignoreRestSiblings": true }],
"@typescript-eslint/no-use-before-define": "off",
"@typescript-eslint/no-empty-function": "off",
'@typescript-eslint/array-type': ['error', { default: 'array-simple' }],
'@typescript-eslint/ban-types': 'off',
'@typescript-eslint/explicit-function-return-type': 'off',
'@typescript-eslint/explicit-member-accessibility': 'off',
'@typescript-eslint/indent': 'off',
'@typescript-eslint/no-empty-interface': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-parameter-properties': 'off',
'@typescript-eslint/no-triple-slash-reference': 'off',
'@typescript-eslint/no-unused-vars': [
'error',
{ argsIgnorePattern: '^_', ignoreRestSiblings: true },
],
'@typescript-eslint/no-use-before-define': 'off',
'@typescript-eslint/no-empty-function': 'off',
// It"s all very well saying that some types are trivially inferrable,
// but being explicit is still clearer.
"@typescript-eslint/no-inferrable-types": "off",
'@typescript-eslint/no-inferrable-types': 'off',
'@typescript-eslint/explicit-module-boundary-types': 'off',
'@typescript-eslint/naming-convention': 'off',
'@typescript-eslint/ban-ts-comment': [
'error',
{
'ts-ignore': 'allow-with-description',
'ts-expect-error': 'allow-with-description',
},
],
},
env: {
jest: true,
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,8 @@
"@types/resize-observer-browser": "^0.1.1",
"@types/tabbable": "^3.1.0",
"@types/uuid": "^3.4.4",
"@typescript-eslint/eslint-plugin": "^2.27.0",
"@typescript-eslint/parser": "^2.27.0",
"@typescript-eslint/eslint-plugin": "^3.2.0",
"@typescript-eslint/parser": "^3.2.0",
"autoprefixer": "^7.1.5",
"axe-core": "^3.3.2",
"axe-puppeteer": "^1.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/components/with_theme/theme_context.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { EUI_THEMES, EUI_THEME } from '../../../../src/themes';
// @ts-ignore
// @ts-ignore importing from a JS file
import { applyTheme } from '../../services';

const defaultState = {
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/color_picker/color_palette_picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
EuiColorPalettePicker,
EuiColorPalettePickerPaletteProps,
} from '../../../../src/components/color_picker/color_palette_picker';
// @ts-ignore
// @ts-ignore importing from a JS file
import { DisplayToggles } from '../form_controls/display_toggles';

const palettes: EuiColorPalettePickerPaletteProps[] = [
Expand Down
2 changes: 1 addition & 1 deletion src-docs/src/views/header/header_stacked.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import { EuiSwitch } from '../../../../src/components/form';
import { EuiSpacer } from '../../../../src/components/spacer';
import { EuiAvatar } from '../../../../src/components/avatar';
// @ts-ignore
// @ts-ignore importing from a JS file
import HeaderUpdates from './header_updates';

export default () => {
Expand Down
3 changes: 1 addition & 2 deletions src/components/bottom_bar/bottom_bar.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,8 @@ import { keysOf } from '../common';

import { EuiBottomBar, paddingSizeToClassNameMap } from './bottom_bar';

// TODO: Temporary hack which we can remove once react-test-renderer supports portals.
// @ts-ignore TODO: Temporary hack which we can remove once react-test-renderer supports portals.
// More info at https://github.com/facebook/react/issues/11565.
// @ts-ignore
ReactDOM.createPortal = node => node;

describe('EuiBottomBar', () => {
Expand Down
6 changes: 3 additions & 3 deletions src/components/code_editor/code_editor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ describe('EuiCodeEditor', () => {
test('should be enabled when the ui ace box loses focus', () => {
const hint = findTestSubject(component, 'codeEditorHint');
hint.simulate('keyup', { key: keys.ENTER });
// @ts-ignore
// @ts-ignore onBlurAce is known to exist and its params are only passed through to the onBlur callback
component.instance().onBlurAce();
expect(
findTestSubject(component, 'codeEditorHint').getDOMNode()
Expand All @@ -106,13 +106,13 @@ describe('EuiCodeEditor', () => {
test('bluring the ace textbox should call a passed onBlur prop', () => {
const blurSpy = jest.fn().mockName('blurSpy');
const el = mount(<EuiCodeEditor onBlur={blurSpy} />);
// @ts-ignore
// @ts-ignore onBlurAce is known to exist and its params are only passed through to the onBlur callback
el.instance().onBlurAce();
expect(blurSpy).toHaveBeenCalled();
});

test('pressing escape in ace textbox will enable overlay', () => {
// @ts-ignore
// @ts-ignore onKeydownAce is known to exist and its params' values are unimportant
component.instance().onKeydownAce({
preventDefault: () => {},
stopPropagation: () => {},
Expand Down
3 changes: 1 addition & 2 deletions src/components/color_picker/color_picker.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ import { VISUALIZATION_COLORS, keys } from '../../services';
import { requiredProps, findTestSubject, sleep } from '../../test';

jest.mock('../portal', () => ({
// @ts-ignore
EuiPortal: ({ children }) => children,
EuiPortal: ({ children }: { children: any }) => children,
}));

const onChange = jest.fn();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ import { EuiColorStopThumb } from './color_stop_thumb';
import { requiredProps } from '../../../test';

jest.mock('../../portal', () => ({
// @ts-ignore
EuiPortal: ({ children }) => children,
EuiPortal: ({ children }: { children: any }) => children,
}));

const onChange = jest.fn();
Expand Down
3 changes: 1 addition & 2 deletions src/components/color_picker/color_stops/color_stops.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ import {
import { requiredProps, findTestSubject } from '../../../test';

jest.mock('../../portal', () => ({
// @ts-ignore
EuiPortal: ({ children }) => children,
EuiPortal: ({ children }: { children: any }) => children,
}));

const onChange = jest.fn();
Expand Down
6 changes: 2 additions & 4 deletions src/components/color_picker/color_stops/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,13 @@ describe('isInvalid', () => {

test('Should mark colorStops missing stop as invalid', () => {
const colorStops = [{ stop: null, color: '#FF0000' }];
// Intentionally wrong
// @ts-ignore
// @ts-ignore Intentionally wrong
expect(isInvalid(colorStops)).toBe(true);
});

test('Should mark colorStops with invalid stop as invalid', () => {
const colorStops = [{ stop: 'I am not a number', color: '#FF0000' }];
// Intentionally wrong
// @ts-ignore
// @ts-ignore Intentionally wrong
expect(isInvalid(colorStops)).toBe(true);
});
});
Expand Down
28 changes: 12 additions & 16 deletions src/components/datagrid/data_grid.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function openColumnSorterSelection(datagrid: ReactWrapper) {
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand All @@ -131,7 +131,7 @@ function closeColumnSorterSelection(datagrid: ReactWrapper) {
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand Down Expand Up @@ -162,7 +162,7 @@ function getColumnSortDirection(
`[data-test-subj="dataGridColumnSortingPopoverColumnSelection-${columnId}"]`
);
expect(selectColumnButton.length).toBe(1);
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => selectColumnButton.props().onClick());

// close column selection popover
Expand Down Expand Up @@ -200,7 +200,7 @@ function openColumnSorter(datagrid: ReactWrapper) {
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand All @@ -223,7 +223,7 @@ function closeColumnSorter(datagrid: ReactWrapper) {
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand Down Expand Up @@ -251,12 +251,8 @@ function sortByColumn(
// if this column isn't being sorted, enable it
if (currentSortDirection === 'off') {
act(() => {
// @ts-ignore-next-line
columnSorter
.find('EuiSwitch')
.props()
// @ts-ignore-next-line
.onChange();
// @ts-ignore does not require an argument in this usage
columnSorter.find('EuiSwitch').props().onChange!();
});

datagrid.update();
Expand Down Expand Up @@ -332,7 +328,7 @@ function openColumnSelector(datagrid: ReactWrapper) {
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand All @@ -355,7 +351,7 @@ function closeColumnSelector(datagrid: ReactWrapper) {
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand Down Expand Up @@ -401,7 +397,7 @@ function moveColumnToIndex(
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand All @@ -418,7 +414,7 @@ function moveColumnToIndex(
const portal = popover.find('EuiPortal');
act(() =>
portal.find('EuiDragDropContext').props().onDragEnd!({
// @ts-ignore-next-line - only `index` is used from `source`, don't need to mock rest of the event
// @ts-ignore - only `index` is used from `source`, don't need to mock rest of the event
source: { index: initialColumnIndex },
destination: { index: nextIndex },
})
Expand All @@ -436,7 +432,7 @@ function moveColumnToIndex(
.find('div[className="euiPopover__anchor"]')
.find('[onClick]')
.first();
// @ts-ignore-next-line
// @ts-ignore onClick is known to exist, and does not require an argument in this usage
act(() => popoverButton.props().onClick());

datagrid.update();
Expand Down
1 change: 0 additions & 1 deletion src/components/datagrid/data_grid_body.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
*/

import React, { Fragment, FunctionComponent, useMemo } from 'react';
// @ts-ignore-next-line
import { EuiCodeBlock } from '../code';
import {
EuiDataGridControlColumn,
Expand Down
4 changes: 2 additions & 2 deletions src/components/datagrid/data_grid_inmemory_renderer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ function noop() {}
function getElementText(element: HTMLElement) {
return 'innerText' in element
? element.innerText
: // TS thinks element.innerText always exists, however it doesn't in jest/jsdom enviornment
// @ts-ignore-next-line
: // (this line left here to satisfy Prettier since a ts-ignore is used on the next line)
// @ts-ignore TypeScript thinks element.innerText always exists, however it doesn't in jest/jsdom enviornment
element.textContent || undefined;
}

Expand Down
2 changes: 0 additions & 2 deletions src/components/datagrid/style_selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
import React, { ReactElement, useState } from 'react';
import { EuiDataGridStyle } from './data_grid_types';
import { EuiI18n } from '../i18n';
// @ts-ignore-next-line
import { EuiPopover } from '../popover';
// @ts-ignore-next-line
import { EuiButtonEmpty, EuiButtonGroup } from '../button';

export const startingStyles: EuiDataGridStyle = {
Expand Down

0 comments on commit e0f79ff

Please sign in to comment.