Skip to content
This repository was archived by the owner on Dec 10, 2021. It is now read-only.

refactor(chart-controls): improve typing and file organization#962

Merged
ktmud merged 4 commits intomasterfrom
shared-controls
Feb 22, 2021
Merged

refactor(chart-controls): improve typing and file organization#962
ktmud merged 4 commits intomasterfrom
shared-controls

Conversation

@ktmud
Copy link
Contributor

@ktmud ktmud commented Feb 17, 2021

🏆 Enhancements
🏠 Internal

Improve typing and file organization for shared controls.

  1. Reuse Metric definition in superset-ui/core.
  2. Move the React components for shared controls to its own folder (prepare for adding more custom controls).
  3. Export empty objects in pure typing files to fix build errors with npm link:
    cannot-read-call

@ktmud ktmud requested a review from a team as a code owner February 17, 2021 03:15
@vercel
Copy link

vercel bot commented Feb 17, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/superset/superset-ui/j0lpc9ulv
✅ Preview: https://superset-ui-git-shared-controls.superset.now.sh

@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #962 (f5bafd5) into master (d433339) will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #962   +/-   ##
=======================================
  Coverage   27.70%   27.70%           
=======================================
  Files         401      401           
  Lines        8255     8255           
  Branches     1143     1143           
=======================================
  Hits         2287     2287           
  Misses       5831     5831           
  Partials      137      137           
Impacted Files Coverage Δ
...controls/src/components/InfoTooltipWithTrigger.tsx 100.00% <ø> (ø)
...-ui-chart-controls/src/components/MetricOption.tsx 53.33% <ø> (ø)
...erset-ui-chart-controls/src/components/Tooltip.tsx 20.00% <ø> (ø)
packages/superset-ui-chart-controls/src/index.ts 100.00% <ø> (ø)
.../shared-controls/components/RadioButtonControl.tsx 9.09% <ø> (ø)
.../superset-ui-core/src/query/types/QueryFormData.ts 100.00% <ø> (ø)
...s/legacy-plugin-chart-time-table/src/TimeTable.tsx 0.00% <0.00%> (ø)

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 d433339...f5bafd5. Read the comment docs.


export { default as __hack_reexport_chart_Base } from './types/Base';
export { default as __hack_reexport_chart_TransformFunction } from './types/TransformFunction';
export { default as __hack_reexport_chart_QueryResponse } from './types/QueryResponse';
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This hack fixes script errors in dev build when re-exporting pure type files:

image

Not sure what is the root cause, but this fixes it.

Copy link

@espressoroaster espressoroaster left a comment

Choose a reason for hiding this comment

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

From an offline conversation with Jesse, export default {}; is a hack solution for re-exporting * from pure type files that don't contain real JavaScript variables.

Jesse explained that there is a failure under certain circumstances that Webpack rebuild will fail and throw an error that files do not exist without the hack.

PR LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants