ESLint: Remove ts-ignore comments #10933
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@rusackas Can you please take a look? |
Codecov Report
@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
There were issues on |
kgabryje
force-pushed
the
eslint/ts-ignore
branch
from
803b24c
to
20259fe
Compare
rusackas
reviewed
Sep 18, 2020
rusackas
reviewed
Sep 18, 2020
| > | ||
| {children} | ||
| </Option> | ||
| <ClassNames> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
reviewed
Sep 18, 2020
| @@ -38,6 +38,8 @@ export type FeatureFlagMap = { | |||
| declare global { | |||
| interface Window { | |||
| featureFlags: FeatureFlagMap; | |||
| $: any; | |||
| jQuery: any; | |||
There was a problem hiding this comment.
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
approved these changes
Sep 18, 2020
|
@kgabryje @rusackas This change is causing failures on master https://github.com/apache/incubator-superset/runs/1146648924 |
|
|
Opened a PR to (at least temporarily) revert that commit. #10987 |
amitmiran137
pushed a commit
to ofekisr/incubator-superset
that referenced
this pull request
Sep 22, 2020
…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) ...
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
* 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
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
I removed some
@ts-ignorecomments and refactored the code accordingly. Some components required a simple dependency bump, installing@typespackage 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 passonPasteprop.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