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

feat: add vega interpreter as dependency and support custom interpreter #475

Closed
wants to merge 2 commits into from

Conversation

domoritz
Copy link
Member

@domoritz domoritz changed the title fix: add vega interpreter as dependency feat: add vega interpreter as dependency and support custom interpreter Jun 15, 2020
@domoritz domoritz requested a review from jheer June 15, 2020 02:46
@domoritz
Copy link
Member Author

Alternatively, I could try to load the expression parser from vega (unless otherwise specified) and expect users who bundle their app to always provide the expression parser directly.

However, it looks like the parser is pretty small and it is nice that we don't require users to do any extra work to use the expression parser.

@domoritz
Copy link
Member Author

Someone should redo this pull request.

@domoritz
Copy link
Member Author

domoritz commented May 5, 2021

Will redo if anyone requests this feature.

@domoritz domoritz closed this May 5, 2021
@stefanw
Copy link

stefanw commented Sep 13, 2021

Hey @domoritz, I would appreciate this feature. Right now, passing ast: true does not help much without a custom vega build because vega.expressionInterpreter needs to exist which it does not when using ES module bundling. I'm currently working around it like this to make sure expr on the vega view is set:

import * as vega from 'vega';
import { expressionInterpreter } from 'vega-interpreter';
import embed from 'vega-embed';

class CSPVegaView extends vega.View {
  constructor (runtime, opt) {
    opt.expr = expressionInterpreter
    super(runtime, opt);
  }
}

// ... define el, spec
embed(el, spec, {
    viewClass: CSPVegaView,
    ast: true
  })

It's a bit cumbersome. I think either shipping vega-embed with vega-interpreter as a dependency or allowing to pass in your own expr option to embed could be a solution.


Also: congratulations on your recent career step! Awesome that you still find the time to work on vega stuff.

@domoritz
Copy link
Member Author

Oh hello. Nice to see you here. I'll put it back in my queue to finish this up.

@domoritz
Copy link
Member Author

I put it in my heap instead.

@domoritz domoritz deleted the dom/interpreter branch September 13, 2021 13:23
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

Successfully merging this pull request may close these issues.

2 participants