Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review Svelte API #4

Closed
domoritz opened this issue Apr 23, 2021 · 7 comments
Closed

Review Svelte API #4

domoritz opened this issue Apr 23, 2021 · 7 comments
Assignees
Milestone

Comments

@domoritz
Copy link
Member

No description provided.

@domoritz domoritz added this to the First release milestone Apr 27, 2021
@domoritz
Copy link
Member Author

One big question for me is error handling. @cabreraalex should we catch errors and provide a handler or just let error bubble up? For example in

@domoritz domoritz mentioned this issue Apr 29, 2021
@cabreraalex
Copy link
Member

Does it use promises? There is built-in error handling for async calls: https://svelte.dev/tutorial/await-blocks

@cabreraalex
Copy link
Member

I guess there's no de-facto way to do real try blocks: sveltejs/svelte#3733

@cabreraalex
Copy link
Member

LGTM, two thoughts:

  1. Does it make sense to just have one Vega component? the vega-embed API doesn't distinguish between vega and vegalite, can take either in and figure out which it is.
  2. I think bubbling up errors to the user in promises is too confusing. If anything, we could have an "error" component that shows a red message when it fails to render. Right now it's just blank. Or let people pass in slots or a custom component to show on error. idk, lots of ways to do it, it's fine as is though

@domoritz
Copy link
Member Author

domoritz commented May 3, 2021

We have a Vega-Embed component. The advantage of Vega/Vega-Lite components is that they set the mode. I think that can be useful. Maybe we could expose the VegaEmbed component?

The advantage of a pure Vega component could be that we skip the Vega-Lite import (not sure whether we are doing that right now but we could).

@domoritz
Copy link
Member Author

domoritz commented May 3, 2021

I'm surprised there is no proper/standard error handling in Svelte. If you think what we have is okay for now, let's release.

@cabreraalex
Copy link
Member

Go for it!

@domoritz domoritz closed this as completed May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants