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

fix(deck.gl): make Deck.gl Javascript tooltip generator works correctly #19059

Conversation

chanyou0311
Copy link
Contributor

@chanyou0311 chanyou0311 commented Mar 8, 2022

SUMMARY

This PR fixed the problem that JAVASCRIPT TOOLTIP GENERATOR of Deck.gl is not working.

I installed vm-browserify and specified it in webpack.config.js.
Because Webpack 5 no longer polyfills and vm module is used at sandboxedEval function defined superset/superset-frontend/plugins/legacy-preset-chart-deckgl/src/utils/sandbox.js.

Webpack 5 no longer polyfills Node.js core modules automatically which means if you use them in your code running in browsers or alike, you will have to install compatible modules from npm and include them yourself.
https://webpack.js.org/configuration/resolve/

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before

before_capture.mp4

After

after_capture.mp4

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

@chanyou0311 chanyou0311 changed the title [WIP]fix(deck.gl): make Deck.gl Javascript tooltip generator works correctly fix(deck.gl): make Deck.gl Javascript tooltip generator works correctly Mar 8, 2022
@zhaoyongjie zhaoyongjie self-requested a review March 8, 2022 10:07
Copy link
Member

@zhaoyongjie zhaoyongjie left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for the fix and for pointing this out.

@codecov
Copy link

codecov bot commented Mar 8, 2022

Codecov Report

Merging #19059 (ca0b57d) into master (e89f0ab) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #19059   +/-   ##
=======================================
  Coverage   66.52%   66.52%           
=======================================
  Files        1641     1641           
  Lines       63476    63476           
  Branches     6444     6444           
=======================================
  Hits        42227    42227           
  Misses      19585    19585           
  Partials     1664     1664           
Flag Coverage Δ
javascript 51.28% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out 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 e89f0ab...ca0b57d. Read the comment docs.

@villebro villebro merged commit 2cc5678 into apache:master Mar 8, 2022
@chanyou0311 chanyou0311 deleted the fix/make-deckgl-javascript-tooltip-generator-work branch March 9, 2022 03:56
villebro pushed a commit that referenced this pull request Apr 3, 2022
@DataMinerR
Copy link

@chanyou0311
The tooltip used to work for me, but since the update, it is throwing the same error like the one reported above.

Your suggested solution seems to be "I installed vm-browserify and specified it in webpack.config.js.". Can you please provide more details? How and where to install vm-browserify and how to specify it in webpack.config.js.

Thanks in advance

@dtung011
Copy link

dtung011 commented Feb 21, 2024

Set TALISMAN_ENABLED = False in superset_config.py will remove minifed react #31 for the tooltip generator.
My version: 3.1

@rusackas
Copy link
Member

Rather than disabling Talisman entirely, I wonder what holes would need to be poked in its configuration to allow this to work without removing all other safeguards.

@mistercrunch mistercrunch added 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.0.0 labels Mar 13, 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 lts-v1 size/XS 🍒 1.5.0 🍒 1.5.1 🍒 1.5.2 🍒 1.5.3 🚢 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants