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

fix(chart-controls): transform emotion css prop #1036

Merged
merged 2 commits into from
Apr 1, 2021
Merged

Conversation

ktmud
Copy link
Contributor

@ktmud ktmud commented Apr 1, 2021

🐛 Bug Fix

Emotion's css prop must be transpiled to use emotion's jsx function. Not doing this is will result in missing styles when the built React components (plugin/lib/../Component.js) are used in superset-frontend. This affect only production builds, and not npm link because npm link uses plugin/src/../Components.tsx, which can be picked up by superset-frontend's Babel emotion plugin.

Tested by temporarily disable src replacement logic in superset-frontend's webpack.config.js.

Related: apache/superset#13758 (comment)

@ktmud ktmud requested a review from a team as a code owner April 1, 2021 17:30
@vercel
Copy link

vercel bot commented Apr 1, 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/4PNrs9chnyGfpR4PYgiiTzrtWfXC
✅ Preview: https://superset-ui-git-babel-emotion-superset.vercel.app

@kristw
Copy link
Contributor

kristw commented Apr 1, 2021

Ah wait. The code looks like it makes sense but the build is failing.

@ktmud
Copy link
Contributor Author

ktmud commented Apr 1, 2021

Ah wait. The code looks like it makes sense but the build is failing.

Just pushed a fix. (Hope it works!)

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #1036 (70426a7) into master (cd632f7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1036   +/-   ##
=======================================
  Coverage   27.74%   27.74%           
=======================================
  Files         428      428           
  Lines        8758     8758           
  Branches     1314     1314           
=======================================
  Hits         2430     2430           
  Misses       6155     6155           
  Partials      173      173           
Impacted Files Coverage Δ
...hart-controls/src/components/ControlForm/index.tsx 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 cd632f7...70426a7. Read the comment docs.

@ktmud ktmud merged commit 6df527c into master Apr 1, 2021
@delete-merged-branch delete-merged-branch bot deleted the babel-emotion branch April 1, 2021 17:58
NejcZdovc pushed a commit to blotoutio/superset-ui that referenced this pull request Apr 20, 2021
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