apache-superset/superset-roadmap

[Dashboard] Render all viz components in iFrames

Closed this issue · 2 comments

Motivation / Problem to Solve

Charts in dashboards currently have a few issues that could use fixing:
• Security enhancements (sandboxing) - it's possible for a chart's JS code to reach into the browser's window object and do just about anything it wants. This isn't really an issue in the current world of curated viz plugins, but as Superset gets closer and closer to a thriving community of plugin developers, this is something we need to get ahead of
• Performance (impact TBD) - many plugins render a potentially huge number of DOM elements, and do a lot of computation around those elements. Most browsers currently render iframes on a single thread, but some (Chromium) seem to be working to speed that up with multithreading. We'll need to do some assessment of the performance pros/cons here, but we'll probably see some wins and some losses.
• Fault tolerance - I may be wrong about this (research needed) but I believe that in a worst-case scenario, a chart that completely bombs out (like the sad crash face) would only crash that iFrame, and not take out the entire page.
• Media query support - This is a bit of a blessing/curse, but I think mostly positive in our use cases. Iframe content sets media queries by their display area in a page... this means we can make charts responsive, and have them draw differently depending on size.

Proposed Functionality

*** How should the proposed solution work? What are the user expectations? Describe the main use cases or user flows. ***
We can use https://github.com/ryanseddon/react-frame-component. And then, we'll see that all the CSS looks terrible so we'll need to move a lot of styles from Superset down into the plugins themselves (perhaps in a shared/reusable manner).

We might want to start turning this on for only certain plugins (by passing a prop), to enable support for them one by one, or turn it off for any built-in (safe) plugins where performance is impacted, and make it mandatory for any dynamically imported (unsafe) plugins.

ktmud commented

I'm almost certain this will be a drawback on performance instead of a step foward. Each iframe needs to load its own assets, create its own JavaScript/CSS context, a copy of the Superset app/rendering sandbox base code, potentially hogging up memory. Not to mention the additional network requests.

As regard to responsiveness, unlike CSS layouts, most interactive visualizations need to manually compute dimensions of different elements in JS, the responsiveness problem is seldom completely solvable by CSS.

IMO, we should double down on creating nicer and more native charts and make it easier to render them natively in other apps, instead of migrating all charts to iframes. In fact, improvements in thesuperset-ui repo over the last couple of years largely comes from the need of embedding charts without iframes.