Build: optimize frontend build configs to improve superset-ui-plugin dev experience

[ktmud]

CATEGORY

• Build / Development Environment

SUMMARY

This PR optimizes Webpack / Babel / TypeScript configs to make local build with webpack-dev-server and npm link less error-prone. It fixes tones of potential pitfalls that are currently breaking the build process. See details in code comments.

Building with source code (including TypeScript) should now work for most visualization plugins out of the box with a simple npm link command (all official plugins have been tested working):

cd incubator-superset/superset-frontend
npm link ~/path/to/superset-ui-plugins/packages/plugin-name
npm run dev-server

The dev server will automatically use ./node_modules/@superset-ui/plugin-name/src to resolve import "@superset-ui/plugin-name", if the src folder exists (normally only when the npm package @superset-ui/plugin-name is linked to the source code repo via npm link).

To link all plugins, just do:

npm link ~/path/to/superset-ui-plugins/packages/*

To unlink, just run npm install again. Relinking is as simple as hitting Ctr + R in Bash/ZSH:

Note this PR also solves some of the problems mentioned in #8638 (e.g., to eliminate the need to manually edit plugin path in MainPreset.js by automatically resolving symlinked plugin packages to <plugin-name>/src).

Limitation

There is still some issues with the process that is preventing hot-reload to work on Chart plugins (probably a bug or misconfiguration related to react-hot-loader or how Superchart renders a viz_type dynamically). The code will successfully build, but the visualization will not rerender (still can't figure out why...). The remedy is to edit superset-frontend/src/chart/Chart.jsx after editing the plugin.

---

This PR also upgrades following packages:

• react
• react-dom
• react-hot-loader
• @babel/plugin-proposal-optional-chaining (new)
• @hot-loader/react-dom (new)
• jest
• @types/react
• @types/react-dom
• ts-jest
• ts-loader
• tslib
• typescript
• webpack-assets-manifest
• webpack-cli
• webpack-dev-server
• webpack-sources

I figure it's just easier to upgrade everything to the latest version other than chasing down a potential bug that may have been fixed in later versions.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

When running npm run dev-server, there will be a notice on which plugins are now using symlink to its source.

TEST PLAN

1. npm link a plugin, either by the commands presented above or follow the instruction here. Make sure you have run yarn && yarn build in the plugin repo after checking it out.
2. Start the Python backend and the webpack-dev-server: npm run dev-server
3. The build should be successful and should have used your updated source code in packages/plugin-name/src.

ADDITIONAL INFORMATION

• Has associated issue:
  • [wip] superset-ui-plugins linking tool for local development #8638
  • fix: cannot assign to read only property exports of object #9319
  • Charts plugins development integration in Superset apache-superset/superset-ui-plugins#342

REVIEWERS

@suddjian @rusackas @kristw @etr2460

[codecov-io]

Codecov Report

Merging #9326 into master will decrease coverage by 0.53%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #9326      +/-   ##
==========================================
- Coverage   59.33%   58.79%   -0.54%     
==========================================
  Files         374      374              
  Lines       12202    12108      -94     
  Branches     2880     2984     +104     
==========================================
- Hits         7240     7119     -121     
+ Misses       4962     4810     -152     
- Partials        0      179     +179

Impacted Files	Coverage Δ
superset-frontend/src/SqlLab/App.jsx	0% <ø> (ø)	⬆️
superset-frontend/src/addSlice/App.jsx	0% <ø> (ø)	⬆️
superset-frontend/src/dashboard/App.jsx	0% <ø> (ø)	⬆️
superset-frontend/src/profile/App.jsx	0% <ø> (ø)	⬆️
superset-frontend/src/welcome/App.jsx	0% <ø> (ø)	⬆️
superset-frontend/src/explore/App.jsx	0% <0%> (ø)	⬆️
superset-frontend/src/explore/index.jsx	0% <0%> (ø)	⬆️
superset-frontend/src/setup/setupApp.ts	0% <0%> (ø)	⬆️
...ntend/src/dashboard/components/PropertiesModal.jsx	6.55% <0%> (-39.28%)	⬇️
...end/src/SqlLab/components/RunQueryActionButton.tsx	71.42% <0%> (-16.58%)	⬇️
... and 79 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 4659883...87ded7f. Read the comment docs.

[etr2460]

a couple comments. also, could you specify some other manual testing you did? i want to make sure sql lab, dashboards, and explore all still work fine since this is a substantial frontend change. maybe also validate CRUD views work as they have different entry points from the rest of the app

[ktmud]

a couple comments. also, could you specify some other manual testing you did? i want to make sure sql lab, dashboards, and explore all still work fine since this is a substantial frontend change. maybe also validate CRUD views work as they have different entry points from the rest of the app

I just clicked on all the major pages (including some CRUD views) and made sure they didn't throw JS error. Did the same for production build with npm run build, too. Was also hoping the CI tests could catch some errors, too.

[rusackas]

Spent some time today testing this locally, and it seems to be working out great! Thank you for this work, I look forward to it being in master and using it day to day.

One question (perhaps a bit of a tangent): Do we still need to point to NVD3's /lib directory in MainPreset.js? It now seems to work fine without that (loading from @superset-ui/legacy-preset-chart-nvd3 directly), and editing stuff in the /src folder now works with this PR, which is awesome!. If that all seems as safe as it appears, the comments about this in MainPreset.js could also be deleted.

Regarding your hot loading limitation, I did see some symptoms of it, but suspect it's just that D3 doesn't redraw on hot-reload unless something triggers in the the D3's data, triggering it lifecycle to run. For example, I tested an edit to the Big Number component, turning the actual number to "hello" and it appears the hot-loading didn't run, but when you mouseover the chart, D3 is triggered to update. When doing a similar test with Treemap, there are no interactions that seem to "kick" D3 into action, so the change doesn't appear, and you need to refresh. I didn't see any cases where editing/saving Chart.jsx made any difference, for what it's worth.

Copying some of your helpful instructions/comments from the Summary into Contributing.md would be helpful for future devs.

Again, thanks for this, it's great work!

[ktmud]

Thanks for testing, @rusackas ! I'm mostly seeing this with ts plugins and suspect it could be related to how modules are imported or hot load hash is computed differently in ts-loader and babel-loader. Might take another dab to find the root cause today, if no avail, we can probably merge this PR as is. I'll update CONTRIBUTING.md.

[kristw]

Do we still need to point to NVD3's /lib directory in MainPreset.js? It now seems to work fine without that (loading from @superset-ui/legacy-preset-chart-nvd3 directly)

If it is working then we don't need /lib any more. This might be some of the very old issues that got resolved over time but the original fix was never removed.

[kristw]

Thank you @rusackas for testing and @ktmud for fighting with all these crazy configs!

[suddjian]

Code LGTM. I also tested dashboards, explore, and sql lab, with and without symlinks, on my local machine and everything seems to work correctly as far as I can tell.

This is a big improvement, thanks!

[rusackas]

I think this is SUPER useful as-is, even if a couple follow-up PRs might be in the works to tie up loose ends mentioned elsewhere above. Just needs docs, and I'd give it the green light.

[ktmud]

In that case, since the CI is now green, feel free to merge it as is. I'll make another PR to update docs and possibly tune the configs even more.

[mistercrunch]

OMG OMG OMG! thanks much @ktmud