fix(deckgl): deckgl unable to load map #17851

zhaoyongjie · 2021-12-22T15:13:39Z

SUMMARY

Deck.gl unable to load map after migrating to the main repo.

This is due to a conflict between the react-map-gl on which legacy-preset-chart-deckgl and legacy-plugin-chart-map-box depend.

This PR is an emergency fix, and we have to upgrade react-map-gl in the future

Current dependency tree for those modules

├─┬ @superset-ui/legacy-plugin-chart-map-box@0.18.25 -> ./plugins/legacy-plugin-chart-map-box
│ ├── @superset-ui/chart-controls@0.18.25 deduped -> ./packages/superset-ui-chart-controls
│ ├── @superset-ui/core@0.18.25 deduped -> ./packages/superset-ui-core
│ ├── mapbox-gl@0.53.1 invalid: "^2.4.0" from plugins/legacy-preset-chart-deckgl
│ ├── prop-types@15.7.2 deduped
│ ├── react-map-gl@4.1.16 invalid: "^6.1.16" from plugins/legacy-preset-chart-deckgl
│ ├── react@16.14.0 deduped
│ ├── supercluster@4.1.1
│ └── viewport-mercator-project@6.2.3

├─┬ @superset-ui/legacy-preset-chart-deckgl@0.4.13 -> ./plugins/legacy-preset-chart-deckgl
│ ├── @math.gl/web-mercator@3.5.6
│ ├── @superset-ui/chart-controls@0.18.25 deduped -> ./packages/superset-ui-chart-controls
│ ├── @superset-ui/core@0.18.25 deduped -> ./packages/superset-ui-core
│ ├── @types/d3-array@2.9.0
│ ├── bootstrap-slider@10.4.0 deduped
│ ├── d3-array@1.2.4 deduped
│ ├── d3-color@1.4.1 deduped
│ ├── d3-scale@3.3.0
│ ├── deck.gl@8.5.2
│ ├── jquery@3.6.0 deduped
│ ├── lodash@4.17.21 deduped
│ ├── mapbox-gl@0.53.1 deduped invalid: "^2.4.0" from plugins/legacy-preset-chart-deckgl
│ ├── moment@2.29.1 deduped
│ ├── mousetrap@1.6.2 deduped
│ ├── prop-types@15.7.2 deduped
│ ├── react-bootstrap-slider@2.1.5
│ ├── react-dom@16.14.0 deduped
│ ├── react-map-gl@4.1.16 deduped invalid: "^6.1.16" from plugins/legacy-preset-chart-deckgl
│ ├── react@16.14.0 deduped
│ ├── underscore@1.12.0
│ ├── urijs@1.19.6 deduped
│ └── xss@1.0.8

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After

Before

TESTING INSTRUCTIONS

CI
Storybook
Local testing

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

zhaoyongjie · 2021-12-22T15:44:25Z

superset-frontend/plugins/legacy-plugin-chart-map-box/package.json

@@ -30,7 +30,6 @@
  "dependencies": {
    "@superset-ui/chart-controls": "0.18.25",
    "@superset-ui/core": "0.18.25",
-    "mapbox-gl": "^0.53.0",


from root src/node_module get this module

Could we move these to peerDependencies to make sure users of the plugin pull in the relevant deps?

mapbox-gl is a dependency from the react-map-gl. The root cause for this issue is that mapbox-gl version is incompatible between legacy-plugin-chart-map-box and legacy-preset-chart-deckgl.

When we removed mapbox-gl in both packages, npm will auto-resolve the conflict. If the user installs those packages as standalone, mapbox-gl will install by default.

legacy-plugin-chart-map-box dependencies tree

(superset) yongjie.zhao@:superset-frontend$ npm ls --depth 1 | grep -A 30 'map-box' .... .... ├─┬ @superset-ui/legacy-plugin-chart-map-box@0.18.25 -> ./plugins/legacy-plugin-chart-map-box │ ├── @superset-ui/chart-controls@0.18.25 deduped -> ./packages/superset-ui-chart-controls │ ├── @superset-ui/core@0.18.25 deduped -> ./packages/superset-ui-core │ ├── prop-types@15.7.2 deduped │ ├─┬ react-map-gl@4.1.16 │ │ ├── @babel/runtime@7.15.4 deduped │ │ ├── mapbox-gl@0.54.1 │ │ ├── mjolnir.js@2.5.0 │ │ ├── prop-types@15.7.2 deduped │ │ ├── react-virtualized-auto-sizer@1.0.2 deduped │ │ ├── react@16.14.0 deduped invalid: "^15.5.4" from node_modules/omnibar │ │ └── viewport-mercator-project@6.2.3 deduped │ ├── react@16.14.0 deduped │ ├─┬ supercluster@4.1.1 │ │ └── kdbush@2.0.1 │ └─┬ viewport-mercator-project@6.2.3 │ ├── @babel/runtime@7.15.4 deduped │ └── gl-matrix@3.3.0 ....

zhaoyongjie · 2021-12-22T15:44:31Z

superset-frontend/plugins/legacy-preset-chart-deckgl/package.json

@@ -32,12 +32,10 @@
    "deck.gl": "8.5.2",
    "jquery": "^3.4.1",
    "lodash": "^4.17.15",
-    "mapbox-gl": "^2.4.0",


from root src/node_module get this module

Echoing @villebro's comment, we may need this to be a peer dependency so that the plugin continues to work in contexts outside of Superset.

the mapbox-gl come from react-map-gl, so we didn't need to add this.

zhaoyongjie · 2021-12-22T15:44:37Z

superset-frontend/plugins/legacy-preset-chart-deckgl/package.json

    "moment": "^2.20.1",
    "mousetrap": "^1.6.1",
    "prop-types": "^15.6.0",
    "react-bootstrap-slider": "2.1.5",
-    "react-map-gl": "^6.1.16",


from root src/node_module get this module

codecov · 2021-12-22T16:07:42Z

Codecov Report

Merging #17851 (e91f70b) into master (9840fbf) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17851      +/-   ##
==========================================
- Coverage   66.98%   66.98%   -0.01%     
==========================================
  Files        1609     1609              
  Lines       64865    64879      +14     
  Branches     6863     6866       +3     
==========================================
+ Hits        43448    43457       +9     
- Misses      19553    19556       +3     
- Partials     1864     1866       +2

Flag	Coverage Δ
javascript	`53.77% <ø> (+<0.01%)`	⬆️

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

Impacted Files	Coverage Δ
...legacy-preset-chart-deckgl/src/DeckGLContainer.jsx	`0.00% <ø> (ø)`
...et-frontend/src/components/EditableTitle/index.tsx	`71.79% <0.00%> (-1.29%)`	⬇️
...end/src/components/Datasource/DatasourceEditor.jsx	`68.93% <0.00%> (-0.17%)`	⬇️
superset-frontend/src/chart/Chart.jsx	`29.62% <0.00%> (ø)`
superset-frontend/src/views/CRUD/alert/types.ts	`100.00% <0.00%> (ø)`
...rset-frontend/src/components/ReportModal/index.tsx	`82.55% <0.00%> (ø)`
...t-frontend/src/explore/reducers/getInitialState.ts	`33.33% <0.00%> (ø)`
...ntend/src/explore/components/ExploreChartPanel.jsx	`14.66% <0.00%> (ø)`
...nd/src/explore/components/ExploreViewContainer.jsx	`2.32% <0.00%> (ø)`
...frontend/src/views/CRUD/alert/AlertReportModal.tsx	`53.96% <0.00%> (+0.42%)`	⬆️
... and 1 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 9840fbf...e91f70b. Read the comment docs.

villebro

Ok makes sense, thanks for the detailed explanation!

* fix(deckgl): deckgl unable to load map * add react-map-gl in peerDependencies

fix(deckgl): deckgl unable to load map

c573dbe

pull-request-size bot added the size/XS label Dec 22, 2021

zhaoyongjie requested review from villebro and rusackas December 22, 2021 15:21

zhaoyongjie commented Dec 22, 2021

View reviewed changes

add react-map-gl in peerDependencies

e91f70b

villebro approved these changes Dec 23, 2021

View reviewed changes

zhaoyongjie merged commit 52f5dcb into apache:master Dec 23, 2021

zhaoyongjie mentioned this pull request Jan 10, 2022

fix: update package list with mapbox-gl #17955

Closed

9 tasks

shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022

fix(deckgl): deckgl unable to load map (apache#17851)

df47758

* fix(deckgl): deckgl unable to load map * add react-map-gl in peerDependencies

bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022

fix(deckgl): deckgl unable to load map (apache#17851)

16b821c

* fix(deckgl): deckgl unable to load map * add react-map-gl in peerDependencies

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(deckgl): deckgl unable to load map #17851

fix(deckgl): deckgl unable to load map #17851

zhaoyongjie commented Dec 22, 2021 •

edited

Loading

zhaoyongjie Dec 22, 2021 •

edited

Loading

villebro Dec 22, 2021

zhaoyongjie Dec 23, 2021 •

edited

Loading

zhaoyongjie Dec 22, 2021 •

edited

Loading

rusackas Dec 22, 2021

zhaoyongjie Dec 23, 2021

zhaoyongjie Dec 22, 2021 •

edited

Loading

codecov bot commented Dec 22, 2021 •

edited

Loading

villebro left a comment

fix(deckgl): deckgl unable to load map #17851

fix(deckgl): deckgl unable to load map #17851

Conversation

zhaoyongjie commented Dec 22, 2021 • edited Loading

SUMMARY

Current dependency tree for those modules

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

After

Before

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

zhaoyongjie Dec 22, 2021 • edited Loading

Choose a reason for hiding this comment

villebro Dec 22, 2021

Choose a reason for hiding this comment

zhaoyongjie Dec 23, 2021 • edited Loading

Choose a reason for hiding this comment

legacy-plugin-chart-map-box dependencies tree

zhaoyongjie Dec 22, 2021 • edited Loading

Choose a reason for hiding this comment

rusackas Dec 22, 2021

Choose a reason for hiding this comment

zhaoyongjie Dec 23, 2021

Choose a reason for hiding this comment

zhaoyongjie Dec 22, 2021 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Dec 22, 2021 • edited Loading

Codecov Report

villebro left a comment

Choose a reason for hiding this comment

zhaoyongjie commented Dec 22, 2021 •

edited

Loading

zhaoyongjie Dec 22, 2021 •

edited

Loading

zhaoyongjie Dec 23, 2021 •

edited

Loading

`legacy-plugin-chart-map-box` dependencies tree

zhaoyongjie Dec 22, 2021 •

edited

Loading

zhaoyongjie Dec 22, 2021 •

edited

Loading

codecov bot commented Dec 22, 2021 •

edited

Loading