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

Retire TSlint #1950

Merged
merged 15 commits into from
May 31, 2019
Merged

Retire TSlint #1950

merged 15 commits into from
May 31, 2019

Conversation

pugnascotia
Copy link
Contributor

This PR retires TSLint in favour of ESLint. Consolidating the linting rules meant I needed to make a call about what the rules should be going forward - I'm very happy to discuss / change them. There's a lot of tedious quote changes, but I think that was unavoidable given the disparate linting configs.

I also expanded the linting / type-checking to cover src-framer. I did my best to patch up the code here, but any tips would be great.

Finally, there seems to be an issue with the scripts/eslint-plugin-i18n/i18n.js - it seems to be exploding on render props that have no arguments, e.g. src/components/steps/step_number.tsx. In the former's case, I can probably make it more robust, but in the latter's case I don't understand why it's using EuiI18n at all - @chandlerprall any ideas?

@pugnascotia
Copy link
Contributor Author

nudge

@thompsongl
Copy link
Contributor

The use of i18n in src/components/steps/step_number.tsx seems to be unwarranted now that the EuiIcon no longer uses title (see #1892 (comment))

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, love it! Have a few comments, but thank you for setting this up.

package.json Outdated Show resolved Hide resolved
"lint-fix": "yarn lint-es-fix && yarn lint-ts-fix",
"lint-es": "eslint --cache --ignore-pattern \"**/*.snap.js\", --ignore-pattern \"**/assets/**/*.js\" \"src/**/*.js\" \"src-docs/**/*.js\"",
"lint-es-fix": "eslint --fix --cache --ignore-pattern \"**/*.snap.js\", \"**/assets/**/*.js\" \"src/**/*.js\" \"src-docs/**/*.js\"",
"lint-es": "eslint --cache '{src,src-docs,src-framer}/**/*.{ts,tsx,js}'",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is so much nicer ❤️

package.json Outdated Show resolved Hide resolved
src/components/combo_box/combo_box.js Outdated Show resolved Hide resolved
src/components/header/header_logo.tsx Outdated Show resolved Hide resolved
@pugnascotia
Copy link
Contributor Author

Ready for another pass 🙏

@chandlerprall
Copy link
Contributor

Errors on build -

tsc --noEmit -p tsconfig-builttypes.json
yarn tsc --noEmit -p tsconfig-builttypes.json

custom_typings/framer/index.d.ts:10:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Boolean, Number, String, FusedNumber, Enum, SegmentedEnum, Color, Image, File, ComponentInstance, Array, Object, Override, OverrideFunction, OverrideObject, animate

10 declare module 'framer' {


eui.d.ts:1:1
 1 declare module '@elastic/eui/src/components/common' {
   ~~~~~~~
 Conflicts are in this file.

custom_typings/framer/index.d.ts:27:5 - error TS2374: Duplicate string index signature.

27     [propName: string]: { type: ControlType; [otherProp: string]: any };
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:1:1 - error TS6200: Definitions of the following identifiers conflict with those in another file: Boolean, Number, String, FusedNumber, Enum, SegmentedEnum, Color, Image, File, ComponentInstance, Array, Object, Override, OverrideFunction, OverrideObject, animate

1 declare module '@elastic/eui/src/components/common' {

custom_typings/framer/index.d.ts:10:1
10 declare module 'framer' {
~~~~~~~
Conflicts are in this file.

eui.d.ts:5908:31 - error TS2307: Cannot find module '@elastic/eui/lib/components/avatar/avatar.js'.

5908 import { SIZES, TYPES } from '@elastic/eui/lib/components/avatar/avatar.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:5932:25 - error TS2307: Cannot find module '@elastic/eui/lib/components/badge/badge.js'.

5932 import { COLORS } from '@elastic/eui/lib/components/badge/badge.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:5933:29 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/index.js'.

5933 import { ICON_TYPES } from '@elastic/eui/lib/components/icon/index.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:5960:32 - error TS2307: Cannot find module '@elastic/eui/lib/components/button/button.js'.

5960 import { COLORS, SIZES } from '@elastic/eui/lib/components/button/button.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:5961:29 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/index.js'.

5961 import { ICON_TYPES } from '@elastic/eui/lib/components/icon/index.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:5992:32 - error TS2307: Cannot find module '@elastic/eui/lib/components/call_out/call_out.js'.

5992 import { COLORS, SIZES } from '@elastic/eui/lib/components/call_out/call_out.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:5993:29 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/index.js'.

5993 import { ICON_TYPES } from '@elastic/eui/lib/components/icon/index.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6019:48 - error TS2307: Cannot find module '@elastic/eui/lib/components/card/card.js'.

6019 import { ALIGNMENTS, LAYOUT_ALIGNMENTS } from '@elastic/eui/lib/components/card/card.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6020:39 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/icon.js'.

6020 import { COLORS, SIZES, TYPES } from '@elastic/eui/lib/components/icon/icon.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6051:44 - error TS2307: Cannot find module '@elastic/eui/lib/components/code/_code_block.js'.

6051 import { FONT_SIZES, PADDING_SIZES } from '@elastic/eui/lib/components/code/_code_block.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6075:49 - error TS2307: Cannot find module '@elastic/eui/lib/components/description_list/description_list.js'.

6075 import { TYPES, ALIGNMENTS, TEXT_STYLES } from '@elastic/eui/lib/components/description_list/description_list.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6104:39 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/icon.js'.

6104 import { COLORS, SIZES, TYPES } from '@elastic/eui/lib/components/icon/icon.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6183:29 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/index.js'.

6183 import { ICON_TYPES } from '@elastic/eui/lib/components/icon/index.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6211:39 - error TS2307: Cannot find module '@elastic/eui/lib/components/icon/icon.js'.

6211 import { COLORS, SIZES, TYPES } from '@elastic/eui/lib/components/icon/icon.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6235:24 - error TS2307: Cannot find module '@elastic/eui/lib/components/panel/panel.js'.

6235 import { SIZES } from '@elastic/eui/lib/components/panel/panel.js';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6257:29 - error TS2307: Cannot find module '@elastic/eui/lib/components/text/text'.

6257 import { TEXT_SIZES } from '@elastic/eui/lib/components/text/text';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6258:29 - error TS2307: Cannot find module '@elastic/eui/lib/components/text/text_align'.

6258 import { ALIGNMENTS } from '@elastic/eui/lib/components/text/text_align';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6259:25 - error TS2307: Cannot find module '@elastic/eui/lib/components/text/text_color'.

6259 import { COLORS } from '@elastic/eui/lib/components/text/text_color';
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

eui.d.ts:6280:46 - error TS2307: Cannot find module '@elastic/eui/lib/components/title/title'.

6280 import { TEXT_TRANSFORM, TITLE_SIZES } from '@elastic/eui/lib/components/title/title';

@pugnascotia
Copy link
Contributor Author

@chandlerprall turns out the framer definitions where being incorporated into the TS build. I've fixed that up now, still waiting for CI to pick up the commit though.

@pugnascotia pugnascotia merged commit 190e6b4 into elastic:master May 31, 2019
@pugnascotia pugnascotia deleted the retire-tslint branch May 31, 2019 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants