[wip] superset-ui-plugins linking tool for local development #8638
Conversation
suddjian
changed the title
Codecov Report
@@ Coverage Diff @@
## master #8638 +/- ##
==========================================
+ Coverage 65.85% 65.91% +0.05%
==========================================
Files 482 482
Lines 24152 24157 +5
Branches 2770 2770
==========================================
+ Hits 15906 15922 +16
+ Misses 8068 8057 -11
Partials 178 178
Continue to review full report at Codecov.
|
|
@williaster mind taking a look at this in lieu of @kristw since he's on leave? |
superset/assets/plugin-devmode.js
Outdated
|
|
||
| const PLUGINS_REPO = | ||
| process.env.SUPERSET_UI_PLUGINS_PATH || | ||
| path.resolve('../../../superset-ui-plugins'); |
There was a problem hiding this comment.
future: eventually we'll have to add support for other repos like https://github.com/apache-superset/superset-ui-plugins-deckgl
|
There's an issue with the approach used here: The code initializing webpack aliases is checking for any linked superset-ui-plugins packages, and modifying the imports to import from This can be fixed by introducing a |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue |
suddjian
changed the title
|
Was anyone tempted to move Currently we haven't migrated off the legacy plugins and the added overhead of syncing package versions, maintaining separate building processes only makes the migration/iterations slower. Both Redash and Metabase didn't take this approach. IMO their visualization registries look much cleaner than Superset and are more developer-friendly to new contributors. |
rusackas
force-pushed
the
package-linking
branch
from
210d681
to
c44d735
Compare
CONTRIBUTING.md
Outdated
| @@ -805,6 +805,12 @@ Here's an example as a Github PR with comments that describe what the | |||
| different sections of the code do: | |||
| https://github.com/apache/incubator-superset/pull/3013 | |||
|
|
|||
| To run Superset using visualization plugins from your local machine, run `npm run plugin-devmode-on`. This will use `npm link` to connect the Superset frontend to your local copy of [superset-ui-plugins](https://github.com/apache-superset/superset-ui-plugins). This command uses the environment variable `SUPERSET_UI_PLUGINS_PATH` to determine the path, defaulting to `../superset-ui-plugins`. So, for examplem, if the path to your local `superset-ui-plugins` repo, relative to `incubator-superset`, happens to be `../../my-fork-of-superset-ui-plugins`, then run `SUPERSET_UI_PLUGINS_PATH=../../my-fork-of-superset-ui-plugins npm run plugin-devmode-on` | |||
rusackas
force-pushed
the
package-linking
branch
from
f630cc5
to
da45f7b
Compare
|
@ktmud I agree that it's been really hard to work on plugins for quite some time now. I'm really hoping we can make it much easier to work with plugins quickly. I've been wondering whether it could make sense to bring back some of the core plugins to the main repo too. The long tail and the community-contributed ones could live in other repos, but having the core in the main repo/package may help. PRing on two repos, publishing packages and bumping versions is tricky, and that's on top of the It's been many times that I want to work in a plugin and either really struggle or completely fail at it. |
|
Closing in favor of #9326 |
CATEGORY
Choose one
SUMMARY
This adds a tool to aid local plugin development/debugging by linking all of the packages in the
superset-ui-pluginsrepo. A new npm script,plugin-devmode-on,npm links all the superset-ui-plugins. A new webpack config detects any npm linked superset-ui-plugins, and automatically changes imports for those plugins to import from/src. This makes a whole series of steps unnecessary. You can set the location to use for the repo with the env variableSUPERSET_UI_PLUGINS_PATH.Another npm script
plugin-devmode-offundoes the changes ofplugin-devmode-onby runningnpm ci.This is not entirely working, just yet. The
BoxPlotChartPluginin@superset-ui/preset-chart-xyplugin is imported by reaching into the/esm/legacydirectory. This breaks, because webpack rewrites it to look forsrc/esm/legacy, which does not exist. We propose to solve this by either modernizing the plugin (no idea how easy/difficult that is), or splitting it into a legacy version and a non-legacy version, so that we don't need to reach inside a build folder to import the chart. Input would be appreciated from anyone with more insight into the history of that plugin.