Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch static viz code changes while running in the dev mode #26015

Merged
merged 9 commits into from
Oct 20, 2022

Conversation

alxnddr
Copy link
Member

@alxnddr alxnddr commented Oct 19, 2022

Changes

FE engineers mostly use yarn dev to run everything or clojure -M:run to run the backend separately locally for development. When working on static viz in order to test updated code engineers needed to rebuild the static viz bundle and then rerun their backend which is not great from the developer experience perspective. This PR adds the yarn build-static-viz:watch command to watch for static viz files changes and update the bundle. Also, it runs as a part of yarn dev.

I'm not using webpack --mode=development because it still minimizes the code. And I'm not using --mode=none which does not minimize the code but it creates a bundle with globals such as process which does not exist in the static viz execution environment.

How to verify

  • Run yarn dev
  • Add a progress bar visualization to a dashboard
  • Subscriptions -> Email -> Send a test email
  • Open frontend/src/metabase/static-viz/components/ProgressBar/utils.ts and change any text that was visible in the test email
  • Send a test email again

Demo

static-viz-update-without-restarting.mov

@alxnddr alxnddr self-assigned this Oct 19, 2022
@codecov
Copy link

codecov bot commented Oct 19, 2022

Codecov Report

Base: 64.20% // Head: 64.20% // No change to project coverage 👍

Coverage data is based on head (c641e9c) compared to base (79bd538).
Patch coverage: 88.88% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #26015   +/-   ##
=======================================
  Coverage   64.20%   64.20%           
=======================================
  Files        3087     3087           
  Lines       91104    91104           
  Branches    11472    11472           
=======================================
  Hits        58494    58494           
  Misses      28020    28020           
  Partials     4590     4590           
Flag Coverage Δ
front-end 44.90% <ø> (ø)

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

Impacted Files Coverage Δ
src/metabase/pulse/render/js_svg.clj 90.81% <88.88%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@alxnddr alxnddr force-pushed the add-static-viz-build-watch-command branch from f982e2d to 0d4b171 Compare October 19, 2022 18:39
@alxnddr alxnddr requested review from a team October 19, 2022 20:00
Copy link
Contributor

@metamben metamben left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

src/metabase/pulse/render/js_svg.clj Outdated Show resolved Hide resolved
test/metabase/pulse/render/js_svg_test.clj Outdated Show resolved Hide resolved
@@ -47,4 +49,7 @@ module.exports = {
"metabase-lib": LIB_SRC_PATH,
},
},
optimization: {
minimize: !shouldDisableMinimization
Copy link
Contributor

Choose a reason for hiding this comment

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

@alxnddr you could use env.WEBPACK_WATCH to disable minification in that case https://webpack.js.org/api/cli/#environment-options

@alxnddr alxnddr merged commit ef09b6a into master Oct 20, 2022
@alxnddr alxnddr deleted the add-static-viz-build-watch-command branch October 20, 2022 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants