ESLint: Remove ts-ignore comments #10933

kgabryje · 2020-09-17T13:04:04Z

SUMMARY

I removed some @ts-ignore comments and refactored the code accordingly. Some components required a simple dependency bump, installing @types package or properly casting the type. In some cases (particularly react-select components) workarounds were necessary - e.g. wrapping an Input component in a div to pass onPaste prop.
There are a few ts-ignores left. I decided to leave them for another PR as they might require some bigger refactors.

TEST PLAN

Run npm run lint, verify there are no new linter errors.

ADDITIONAL INFORMATION

kgabryje · 2020-09-17T13:04:34Z

@rusackas Can you please take a look?

codecov-commenter · 2020-09-17T13:43:59Z

Codecov Report

Merging #10933 into master will decrease coverage by 6.03%.
The diff coverage is 42.85%.

@@            Coverage Diff             @@
##           master   #10933      +/-   ##
==========================================
- Coverage   65.84%   59.81%   -6.04%     
==========================================
  Files         815      781      -34     
  Lines       38335    37222    -1113     
  Branches     3601     3353     -248     
==========================================
- Hits        25243    22265    -2978     
- Misses      12990    14776    +1786     
- Partials      102      181      +79

Flag	Coverage Δ
#cypress	`56.78% <42.85%> (+0.06%)`	⬆️
#javascript	`?`
#python	`61.46% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...et-frontend/src/SqlLab/components/LimitControl.tsx	`65.78% <0.00%> (-22.85%)`	⬇️
...src/components/FilterableTable/FilterableTable.tsx	`2.61% <0.00%> (-83.03%)`	⬇️
superset-frontend/src/components/Link.tsx	`7.69% <ø> (-79.81%)`	⬇️
...erset-frontend/src/components/ListView/Filters.tsx	`84.74% <ø> (-4.81%)`	⬇️
...ponents/Select/WindowedSelect/WindowedMenuList.tsx	`96.29% <ø> (-0.26%)`	⬇️
...perset-frontend/src/datasource/DatasourceModal.tsx	`81.08% <ø> (-14.67%)`	⬇️
...rontend/src/explore/components/PropertiesModal.tsx	`45.76% <0.00%> (-16.36%)`	⬇️
superset-frontend/src/featureFlags.ts	`100.00% <ø> (ø)`
superset-frontend/src/setup/setupApp.ts	`28.57% <ø> (+3.57%)`	⬆️
...perset-frontend/src/views/CRUD/welcome/Welcome.tsx	`0.00% <0.00%> (-85.00%)`	⬇️
... and 265 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5623cd6...dc0c007. Read the comment docs.

mistercrunch · 2020-09-18T04:49:56Z

There were issues on master earlier, please rebase!

superset-frontend/src/components/Select/SupersetStyledSelect.tsx

rusackas · 2020-09-18T20:03:43Z

superset-frontend/src/components/Select/styles.tsx

-    >
-      {children}
-    </Option>
+    <ClassNames>


This is cool, and looks like it'll work... but I'm wondering if there's any advantage to this approach vs the const Styles = styled.div`...` approach

Hmm, I think this approach might be a bit cleaner. If we used styled.div we'd also need to use react-select's API to get default styles and merge them with our data.styles, so in my opinion it's not worth it. WDYT?

rusackas · 2020-09-18T20:08:36Z

superset-frontend/src/featureFlags.ts

@@ -38,6 +38,8 @@ export type FeatureFlagMap = {
 declare global {
  interface Window {
    featureFlags: FeatureFlagMap;
+    $: any;
+    jQuery: any;


I'm not worried about, this, and it's not worth spending any real time on, but curious if https://www.npmjs.com/package/@types/jquery might provide easy types for these

rusackas

LGTM, but wondering if any of those questions are worth addressing before merging.

nytai · 2020-09-22T01:10:34Z

@kgabryje @rusackas This change is causing failures on master https://github.com/apache/incubator-superset/runs/1146648924

This reverts commit a8f5029.

rusackas · 2020-09-22T01:18:41Z

2020-09-21T22:49:21.6155663Z > eslint --ignore-path=.eslintignore --ext .js,.jsx,.ts,.tsx . && npm run type
2020-09-21T22:49:21.6156105Z 
2020-09-21T22:50:19.3926195Z 
2020-09-21T22:50:19.3929001Z /home/runner/work/incubator-superset/incubator-superset/superset-frontend/spec/javascripts/components/TableSelector_spec.jsx
2020-09-21T22:50:19.3930832Z   179:5  warning  Skipped test  jest/no-disabled-tests
2020-09-21T22:50:19.3932051Z   221:5  warning  Skipped test  jest/no-disabled-tests
2020-09-21T22:50:19.3932744Z 
2020-09-21T22:50:19.3934154Z /home/runner/work/incubator-superset/incubator-superset/superset-frontend/spec/javascripts/explore/utils_spec.jsx
2020-09-21T22:50:19.3935922Z    59:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3939217Z    68:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3940312Z    80:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3941191Z    92:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3942061Z   104:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3942930Z   116:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3943799Z   195:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3944281Z 
2020-09-21T22:50:19.3945490Z /home/runner/work/incubator-superset/incubator-superset/superset-frontend/spec/javascripts/sqllab/TableElement_spec.jsx
2020-09-21T22:50:19.3946832Z   52:3  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3947260Z 
2020-09-21T22:50:19.3948419Z /home/runner/work/incubator-superset/incubator-superset/superset-frontend/spec/javascripts/sqllab/actions/sqlLab_spec.js
2020-09-21T22:50:19.3949738Z   110:5  warning  Skipped test  jest/no-disabled-tests
2020-09-21T22:50:19.3950579Z   178:5  warning  Skipped test  jest/no-disabled-tests
2020-09-21T22:50:19.3950980Z 
2020-09-21T22:50:19.3952147Z /home/runner/work/incubator-superset/incubator-superset/superset-frontend/spec/javascripts/sqllab/reducers/sqlLab_spec.js
2020-09-21T22:50:19.3953464Z   247:5  warning  Test has no assertions  jest/expect-expect
2020-09-21T22:50:19.3953857Z 
2020-09-21T22:50:19.3955078Z /home/runner/work/incubator-superset/incubator-superset/superset-frontend/src/views/CRUD/data/savedquery/SavedQueryList.tsx
2020-09-21T22:50:19.3956720Z   31:3  warning  'addDangerToast' is defined but never used   @typescript-eslint/no-unused-vars
2020-09-21T22:50:19.3958073Z   32:3  warning  'addSuccessToast' is defined but never used  @typescript-eslint/no-unused-vars
2020-09-21T22:50:19.3958698Z 
2020-09-21T22:50:19.3959246Z ✖ 15 problems (0 errors, 15 warnings)
2020-09-21T22:50:19.3959572Z 
2020-09-21T22:50:20.2057788Z 
2020-09-21T22:50:20.2059601Z > [email protected] type /home/runner/work/incubator-superset/incubator-superset/superset-frontend
2020-09-21T22:50:20.2060451Z > tsc --noEmit
2020-09-21T22:50:20.2060663Z 
2020-09-21T22:50:44.1527383Z �[96msrc/components/ListView/Filters.tsx�[0m:�[93m140�[0m:�[93m11�[0m - �[91merror�[0m�[90m TS2322: �[0mType '{ label: string; value: string; }' is not assignable to type 'ValueType<GroupType<GroupType<never>>>'.
2020-09-21T22:50:44.1529417Z   Property 'options' is missing in type '{ label: string; value: string; }' but required in type 'GroupType<GroupType<never>>'.
2020-09-21T22:50:44.1530168Z 
2020-09-21T22:50:44.1530855Z �[7m140�[0m           value={selectedOption}
2020-09-21T22:50:44.1531543Z �[7m   �[0m �[91m          ~~~~~�[0m
2020-09-21T22:50:44.1531916Z 
2020-09-21T22:50:44.1533246Z   �[96mnode_modules/@types/react-select/src/types.d.ts�[0m:�[93m11�[0m:�[93m3�[0m
2020-09-21T22:50:44.1534417Z     �[7m11�[0m   options: OptionsType<OptionType>;
2020-09-21T22:50:44.1537461Z     �[7m  �[0m �[96m  ~~~~~~~�[0m
2020-09-21T22:50:44.1538732Z     'options' is declared here.
2020-09-21T22:50:44.1539523Z   �[96mnode_modules/@types/react-select/src/Select.d.ts�[0m:�[93m205�[0m:�[93m3�[0m
2020-09-21T22:50:44.1540192Z     �[7m205�[0m   value?: ValueType<OptionType>;
2020-09-21T22:50:44.1540648Z     �[7m   �[0m �[96m  ~~~~~�[0m
2020-09-21T22:50:44.1541964Z     The expected type comes from property 'value' which is declared here on type 'IntrinsicAttributes & Props<GroupType<GroupType<never>>> & UseAsyncPaginateParams<GroupType<GroupType<never>>, any> & ComponentProps & { ...; } & { ...; }'
2020-09-21T22:50:44.1542896Z 
2020-09-21T22:50:44.1544256Z �[96msrc/components/ListView/Filters.tsx�[0m:�[93m141�[0m:�[93m11�[0m - �[91merror�[0m�[90m TS2322: �[0mType '(selected: SelectOption | null) => void' is not assignable to type '(value: ValueType<GroupType<GroupType<never>>>, action: ActionMeta<GroupType<GroupType<never>>>) => void'.
2020-09-21T22:50:44.1545588Z   Types of parameters 'selected' and 'value' are incompatible.
2020-09-21T22:50:44.1546437Z     Type 'ValueType<GroupType<GroupType<never>>>' is not assignable to type 'SelectOption | null'.
2020-09-21T22:50:44.1547291Z       Type 'undefined' is not assignable to type 'SelectOption | null'.
2020-09-21T22:50:44.1547649Z 
2020-09-21T22:50:44.1548063Z �[7m141�[0m           onChange={onChange}
2020-09-21T22:50:44.1548491Z �[7m   �[0m �[91m          ~~~~~~~~�[0m
2020-09-21T22:50:44.1548650Z 
2020-09-21T22:50:44.1549206Z   �[96mnode_modules/@types/react-select/src/Select.d.ts�[0m:�[93m169�[0m:�[93m3�[0m
2020-09-21T22:50:44.1550056Z     �[7m169�[0m   onChange?: (value: ValueType<OptionType>, action: ActionMeta<OptionType>) => void;
2020-09-21T22:50:44.1550753Z     �[7m   �[0m �[96m  ~~~~~~~~�[0m
2020-09-21T22:50:44.1551913Z     The expected type comes from property 'onChange' which is declared here on type 'IntrinsicAttributes & Props<GroupType<GroupType<never>>> & UseAsyncPaginateParams<GroupType<GroupType<never>>, any> & ComponentProps & { ...; } & { ...; }'
2020-09-21T22:50:44.1552741Z 
2020-09-21T22:50:44.1554123Z �[96msrc/components/ListView/Filters.tsx�[0m:�[93m142�[0m:�[93m11�[0m - �[91merror�[0m�[90m TS2322: �[0mType '(inputValue: string, loadedOptions: SelectOption[], { page }: { page: number; }) => Promise<{ options: { label: string; value: string; }[]; hasMore: boolean; additional: { page: number; }; }>' is not assignable to type 'LoadOptions<GroupType<GroupType<never>>, any>'.
2020-09-21T22:50:44.1555467Z   Types of parameters 'loadedOptions' and 'options' are incompatible.
2020-09-21T22:50:44.1556260Z     Type 'OptionsList<GroupType<GroupType<never>>>' is not assignable to type 'SelectOption[]'.
2020-09-21T22:50:44.1557211Z       The type 'OptionsType<GroupType<GroupType<never>>>' is 'readonly' and cannot be assigned to the mutable type 'SelectOption[]'.
2020-09-21T22:50:44.1557691Z 
2020-09-21T22:50:44.1558183Z �[7m142�[0m           loadOptions={fetchAndFormatSelects}
2020-09-21T22:50:44.1558686Z �[7m   �[0m �[91m          ~~~~~~~~~~~�[0m
2020-09-21T22:50:44.1558828Z 
2020-09-21T22:50:44.1559425Z   �[96mnode_modules/react-select-async-paginate/ts/types.d.ts�[0m:�[93m44�[0m:�[93m5�[0m
2020-09-21T22:50:44.1560143Z     �[7m44�[0m     loadOptions: LoadOptions<OptionType>;
2020-09-21T22:50:44.1560590Z     �[7m  �[0m �[96m    ~~~~~~~~~~~�[0m
2020-09-21T22:50:44.1561767Z     The expected type comes from property 'loadOptions' which is declared here on type 'IntrinsicAttributes & Props<GroupType<GroupType<never>>> & UseAsyncPaginateParams<GroupType<GroupType<never>>, any> & ComponentProps & { ...; } & { ...; }'
2020-09-21T22:50:44.1562604Z 
2020-09-21T22:50:44.1562739Z 
2020-09-21T22:50:44.1562943Z Found 3 errors.
2020-09-21T22:50:44.1563099Z 
2020-09-21T22:50:44.2013547Z npm ERR! code ELIFECYCLE
2020-09-21T22:50:44.2014256Z npm ERR! errno 1
2020-09-21T22:50:44.2048042Z npm ERR! [email protected] type: `tsc --noEmit`
2020-09-21T22:50:44.2048746Z npm ERR! Exit status 1
2020-09-21T22:50:44.2049520Z npm ERR! 
2020-09-21T22:50:44.2050518Z npm ERR! Failed at the [email protected] type script.
2020-09-21T22:50:44.2051630Z npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
2020-09-21T22:50:44.2134011Z 
2020-09-21T22:50:44.2134573Z npm ERR! A complete log of this run can be found in:
2020-09-21T22:50:44.2135568Z npm ERR!     /home/runner/.npm/_logs/2020-09-21T22_50_44_204Z-debug.log
2020-09-21T22:50:44.2196844Z npm ERR! code ELIFECYCLE
2020-09-21T22:50:44.2197185Z npm ERR! errno 1
2020-09-21T22:50:44.2234404Z npm ERR! [email protected] lint: `eslint --ignore-path=.eslintignore --ext .js,.jsx,.ts,.tsx . && npm run type`
2020-09-21T22:50:44.2235089Z npm ERR! Exit status 1
2020-09-21T22:50:44.2235443Z npm ERR! 
2020-09-21T22:50:44.2236440Z npm ERR! Failed at the [email protected] lint script.
2020-09-21T22:50:44.2237161Z npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
2020-09-21T22:50:44.2381461Z 
2020-09-21T22:50:44.2382258Z npm ERR! A complete log of this run can be found in:
2020-09-21T22:50:44.2383564Z npm ERR!     /home/runner/.npm/_logs/2020-09-21T22_50_44_223Z-debug.log
2020-09-21T22:50:44.2420245Z ##[error]Process completed with exit code 1.

rusackas · 2020-09-22T01:19:33Z

Opened a PR to (at least temporarily) revert that commit. #10987

…l_access/dashboard_by_id_endpoints * upstream/master: (29 commits) fix(presto): default unknown types to string type (apache#10753) feat(row-level-security): add base filter type and filter grouping (apache#10946) docs: add gallery screenshot & link in README (apache#10988) docs: add a "Gallery" page (apache#10968) build: add PR lint action (apache#10990) adding filters back that caused issues (apache#10989) chore: selectors refactor in SQLLab test suite (Cypress) (apache#10944) ESLint: Remove ts-ignore comments (apache#10933) style: fix checkbox color (apache#10970) fix: changed disabled rules in datasets module (apache#10979) fix: update the time filter for 'Last Year' option in explore (apache#10829) fix: use nullpool even for user lookup in the celery (apache#10938) Allow empty observations in alerting (apache#10939) fix: re-enabling several globally disabled lint rules (apache#10957) fix: setting specific exceptions common/query_context.py (apache#10942) Pylint disabled rule `pointless-string-statement` is not raising warining anymore - removing (apache#10975) fix: pylint disabled rules in dashboard/api.py (apache#10976) fix: removed disabled lint rule `too-many-locals` in connectors/base/models.py (apache#10958) ESLint: Re-enable rule no-access-state-in-setstate (apache#10870) fix: simply is_adhoc_metric (apache#10964) ...

* Upgrade json-bigint, add @types/json-bigint to deps * Upgrade react-bootstrap-dialog * Fix ts-ignore * Fix ts-ignore * Fix ts-ignore * Upgrade react-syntax-highlighter, fix ts-ignore * Fix ts-ignore * Fix ts-ignore in styles.tsx * Wrap Input in div to pass onPaste * Change esm to cjs imports for highlighter to fix tests * Add null checks

superset-github-bot bot added the polidea label Sep 17, 2020

pull-request-size bot added the size/L label Sep 17, 2020

kgabryje closed this Sep 17, 2020

kgabryje reopened this Sep 17, 2020

rusackas requested a review from suddjian September 17, 2020 16:44

kgabryje added 11 commits September 18, 2020 08:07

Upgrade json-bigint, add @types/json-bigint to deps

9f4e998

Upgrade react-bootstrap-dialog

2028650

Fix ts-ignore

29eb3a9

Fix ts-ignore

0b45175

Fix ts-ignore

af7b8df

Upgrade react-syntax-highlighter, fix ts-ignore

a8fa59f

Fix ts-ignore

6d37ecd

Fix ts-ignore in styles.tsx

9003785

Wrap Input in div to pass onPaste

5d5d3f7

Change esm to cjs imports for highlighter to fix tests

7a50789

Add null checks

20259fe

kgabryje force-pushed the eslint/ts-ignore branch from 803b24c to 20259fe Compare September 18, 2020 06:08

kgabryje closed this Sep 18, 2020

kgabryje reopened this Sep 18, 2020

kgabryje mentioned this pull request Sep 18, 2020

ESLint: Remove ts-ignore #10961

Merged

6 tasks

rusackas reviewed Sep 18, 2020

View reviewed changes

superset-frontend/src/components/Select/SupersetStyledSelect.tsx Show resolved Hide resolved

rusackas reviewed Sep 18, 2020

View reviewed changes

rusackas approved these changes Sep 18, 2020

View reviewed changes

rusackas merged commit a8f5029 into apache:master Sep 21, 2020

rusackas added a commit that referenced this pull request Sep 22, 2020

Revert "ESLint: Remove ts-ignore comments (#10933)"

dbbe1b5

This reverts commit a8f5029.

rusackas mentioned this pull request Sep 22, 2020

Revert "ESLint: Remove ts-ignore comments" #10987

Closed

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.38.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESLint: Remove ts-ignore comments #10933

ESLint: Remove ts-ignore comments #10933

kgabryje commented Sep 17, 2020

kgabryje commented Sep 17, 2020

codecov-commenter commented Sep 17, 2020

mistercrunch commented Sep 18, 2020

rusackas Sep 18, 2020

kgabryje Sep 21, 2020

rusackas Sep 18, 2020

rusackas left a comment

nytai commented Sep 22, 2020

rusackas commented Sep 22, 2020

rusackas commented Sep 22, 2020 •

edited

Loading

ESLint: Remove ts-ignore comments #10933

ESLint: Remove ts-ignore comments #10933

Conversation

kgabryje commented Sep 17, 2020

SUMMARY

TEST PLAN

ADDITIONAL INFORMATION

kgabryje commented Sep 17, 2020

codecov-commenter commented Sep 17, 2020

Codecov Report

mistercrunch commented Sep 18, 2020

rusackas Sep 18, 2020

Choose a reason for hiding this comment

kgabryje Sep 21, 2020

Choose a reason for hiding this comment

rusackas Sep 18, 2020

Choose a reason for hiding this comment

rusackas left a comment

Choose a reason for hiding this comment

nytai commented Sep 22, 2020

rusackas commented Sep 22, 2020

rusackas commented Sep 22, 2020 • edited Loading

rusackas commented Sep 22, 2020 •

edited

Loading