[GitHub] kristw opened a new issue #5680: [SIP-5] [Embeddable Charts] Decouple charts from `slice` and `formData`

[GitBox <gi...@apache.org>]

kristw opened a new issue #5680: [SIP-5] [Embeddable Charts] Decouple charts from `slice` and `formData`
URL: https://github.com/apache/incubator-superset/issues/5680
 
 
   @williaster @conglei @mistercrunch @betodealmeida @hughhhh
   
   cc @john-bodley @michellethomas @graceguo-supercat @timifasubaa
   
   ## [Embeddable Charts] Decouple charts from `slice` and `formData`
   
   ### Motivation
   
   One key part of the embeddable project is to move towards *chart plugins system*, which we can register only necessary charts for superset or register custom ones as wish. This will give more flexibility to the developers to customize their superset instances (making it more lightweight, include-only-needed, or include custom vis type) as well as improve maintainability of the superset codebase.
   
   In order to do that, we aim to split the chart types (i.e. most of the things in `src/visualizations`)  into one or more plugins (npm packages), independent from superset. Then, we will implement a registry mechanism for importing plugins. However, before getting to that points, there are a few issues:
   
   - For the plugin system to work, each chart should be independent from superset platform. Current chart codes are tightly coupled with `slice`. The charts are technically a child of the `slice`, but they take the parent as argument and call utility functions from `slice` (mostly jquery wrapper, which can be replaced with native js). It should to be more one-way dataflow and dispatch events that parent can pick-up rather than calling parents. 
   - The `formData` is a big object that contains many fields, very different from one chart to another. It should be clearer what fields each chart needs. 
   - Some charts are React components, but instead of using it in the slice directly, the current code contructs an empty `<div>` then call `ReactDOM.render()` on the `<div>`. This was done to support the non-React components. This may be not addressed yet in this SIP but the refactored chart code will enable us to remove this complicated logic. 
   
   ### Proposed change. 
   
   The goal of this SIP is to make each chart decoupled, ready to be converted into an independent React component, before separating them into plugins. And we do this cleanup early so we can continue to take improvements/bugfixes from the community while we replace the core part to use plugin system. 
   
   ### New of Changed Public Interfaces
   
   Current chart code are in the form
   
   ```js
   function chart(slice, formData) {
   }
   ```
   
   We will change it to 
   
   ```js
   const propTypes = ...;
   
   function chart(element, props) {
     PropTypes.checkPropTypes(propTypes, props);
     const { data, numberFormat, colorScheme, ... } = props;
     // old code without calls to slice.xxx or formData.xxx
   }
   
   chart.propTypes = propTypes;
   
   function adaptor(slice, formData) {
     const element = document.querySelector(slice.selector);
     return chart(element, { 
       data: ...
       numberFormat: ...
       xxx: ...
     })
   }
   ```
   
   Eventually the `adaptor` will be broken into their own files or adapted to other formats. First pass will keep them in the same file to avoid merge conflicts at `index.js`.
   
   Please see more concrete example in the "Work in progress" section below.
   
   ### New dependencies
   
   None
   
   ### Work in progress
   
   PR | Status | Description
   -- | -- | -- 
   #5669 | Waiting for review | Word cloud
   #5670 | WIP | Tree Map
   #5671 | Waiting for review | Chord Vis
   #5672 | WIP | iframe and markup
   
   
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org