Skip to content

Conversation

@ktmud
Copy link
Member

@ktmud ktmud commented Mar 8, 2021

SUMMARY

Converting remaining functions in controlUtils to TypeScript.

Also updated superset-ui/chart-controls to v0.17.17 to bring in: apache-superset/superset-ui#998

As a side effect, following changes in superset-ui are also brought in:

cc @zhaoyongjie @villebro

Related: #13401

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

N/A

TEST PLAN

No functional changes. CI should pass.

ADDITIONAL INFORMATION

@ktmud ktmud force-pushed the ts-control-utils branch from 3091230 to 11cfb19 Compare March 9, 2021 21:02
@junlincc junlincc requested review from nytai and suddjian March 10, 2021 00:37
@ktmud ktmud force-pushed the ts-control-utils branch 3 times, most recently from 5ccc0d7 to c313e2b Compare March 10, 2021 05:12
@codecov
Copy link

codecov bot commented Mar 10, 2021

Codecov Report

Merging #13520 (a1df6a5) into master (5fca19d) will decrease coverage by 3.71%.
The diff coverage is 91.89%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13520      +/-   ##
==========================================
- Coverage   77.53%   73.82%   -3.72%     
==========================================
  Files         904      602     -302     
  Lines       45986    21304   -24682     
  Branches     5552     5516      -36     
==========================================
- Hits        35657    15728   -19929     
+ Misses      10195     5442    -4753     
  Partials      134      134              
Flag Coverage Δ
cypress 57.45% <75.00%> (+0.25%) ⬆️
hive ?
javascript 63.60% <89.18%> (+0.22%) ⬆️
mysql ?
postgres ?
presto ?
python ?
sqlite ?

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

Impacted Files Coverage Δ
...t-frontend/src/explore/reducers/getInitialState.ts 100.00% <ø> (ø)
...ontend/src/explore/controlUtils/getControlState.ts 95.71% <88.46%> (ø)
...ntend/src/explore/controlUtils/getControlConfig.ts 100.00% <100.00%> (+4.00%) ⬆️
...uperset-frontend/src/explore/controlUtils/index.ts 100.00% <100.00%> (ø)
...set-frontend/src/dashboard/util/getDropPosition.js 90.90% <0.00%> (-1.52%) ⬇️
superset-frontend/src/common/components/index.tsx 100.00% <0.00%> (ø)
superset-frontend/src/components/Button/index.tsx 100.00% <0.00%> (ø)
superset/examples/__init__.py
superset/models/reports.py
superset/annotation_layers/commands/delete.py
... and 311 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 5fca19d...5268258. Read the comment docs.

@ktmud ktmud force-pushed the ts-control-utils branch from 69d80ea to 5268258 Compare March 11, 2021 20:19
);
}

const getMemoizedControlConfig = memoizeOne(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this one just changing location?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, because ESLint complains function being used before definition.

});

it('find control as a control object', () => {
let controlItem = findControlItem(
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we do let controlItem: CustomControlItem?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately no, because findControlItem may also return null or ReactElement. Using simple type declaration will throw an error:

Screen Shot 2021-03-11 at 5 18 26 PM

@ktmud ktmud merged commit 7656b2e into apache:master Mar 12, 2021
@ktmud ktmud deleted the ts-control-utils branch March 12, 2021 01:19
allanco91 pushed a commit to allanco91/superset that referenced this pull request May 21, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 First shipped in 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 1.2.0 First shipped in 1.2.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants