Allow Kino.JS.Live outputs to be exported #321
Conversation
| ## Options | ||
|
|
||
| * `:export` - a function called to export the given kino to Markdown. | ||
| This works the same as `Kino.JS.new/3`, except the function | ||
| receives `t:Kino.JS.Live.Context.t/0` as an argument |
There was a problem hiding this comment.
@josevalim I made this an option instead of a callback, for two reasons:
-
It better mirrors
Kino.JS, so should be more intuitive. We also have the option onKino.Table, this way they are all consistent. -
In some cases the original data may not be passed to the server directly (see Kino.DataTable.new/2). Passing it to the state for the purpose of export is not different from closure, but if it's large data then it may be better to just call
inspect/1right away and close over that.
There was a problem hiding this comment.
Also, I'm not sure if should actually be passing the state to that function. If we do that, then people may make the export reflect the state (e.g. page when paginating the table). It is undesired, because (a) we wouldn't know that we should re-save when the page changes, so it would end up pretty arbitrary (b) it wouldn't reflect the actual value, that is, if the cell returns df and we suddenly persist 10th page, it's weird. Thoughts?
| IO.warn( | ||
| "passing :export_info_string to Kino.JS.new/3 is deprecated. Specify an :export function instead" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Will we have to update many kinos or only Kino.VegaLite?
There was a problem hiding this comment.
Kino.Maplibre too!
There was a problem hiding this comment.
The only other one I found is kino_wardely.
There was a problem hiding this comment.
LGTM! It is only a question if we want to deprecate it now or in a future release. If we deprecate it now, we will have to release new packages for Kino.VegaLite that depend on latest Kino immediately. If it is only Kino.VegaLite it is fine, but if there are more, maybe we want to wait.
|
@josevalim I think it's fine, the new kino brings important changes like support for files and the changed output format, so pushing for newer version makes sense. |
|
Did we update our Kinos according to this? |
|
Not yet. We need to update |
|
Actually I will add a conditional to |
| """ | ||
| @spec new(module(), term(), keyword()) :: t() | ||
| def new(module, data, opts \\ []) do | ||
| # TODO: remove the old export options in Kino v0.14.0 | ||
| export = | ||
| if info_string = opts[:export_info_string] do |
Also improves the export API for
Kino.JS.