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

[Suggestion] Make Renderer.Backends a plugin system #827

Closed
silicakes opened this issue Dec 30, 2020 · 0 comments · Fixed by #1161
Closed

[Suggestion] Make Renderer.Backends a plugin system #827

silicakes opened this issue Dec 30, 2020 · 0 comments · Fixed by #1161
Labels

Comments

@silicakes
Copy link

silicakes commented Dec 30, 2020

Following a discussion happened here: #338

The suggested ways of just hacking around the DOM inside React's lifecycle hooks actually has no real benefits other than syntactic sugar with some (very) naive automagic rendering.

Since the entire DOM management is delegated to VexFlow, we're kinda missing out on its performance benefits (i.e getting all of this into the virtual DOM).

I'm new to this library, but I think that the right approach should be to create a 'React render context' of sorts and manage the data model via state calls.

Since context is currently hard coded, I was also wondering if we can inject external contexts and register it within the renderer itself.

It seems like a small PR looking at the current implementation :

if (this.backend === Renderer.Backends.CANVAS) {

Taking this:

   if (this.backend === Renderer.Backends.CANVAS) {
      // Create context.
      if (!this.element.getContext) {
        throw new Vex.RERR('BadElement', `Can't get canvas context from element: ${elementId}`);
      }
      this.ctx = Renderer.bolsterCanvasContext(this.element.getContext('2d'));
    } else if (this.backend === Renderer.Backends.RAPHAEL) {
      this.ctx = new RaphaelContext(this.element);
    } else if (this.backend === Renderer.Backends.SVG) {
      this.ctx = new SVGContext(this.element);
    } else {
      throw new Vex.RERR('InvalidBackend', `No support for backend: ${this.backend}`);
    }

and making it into

const backend = Renderer.Backends[this.backend];
if(backend) {
  this.ctx = backend.createContext(this.element); // implemented individually within each context
} else {
  throw new Vex.RERR('InvalidBackend', `No support for backend: ${this.backend}`);
}

This should come with something like Renderer.Backends.register(context) and will allow people to create independent plugins for various rendering scenarios.

If that's something you find interesting, I can submit a PR using D3 to show how this might work.

As for phase 2 for this, we can create a small 'Renderer Backend plugin boilerplate' repo with all the relevant test suits so that people will have confidence with their implementations and will be able to update their code for new features of either VexFlow or their engine of choice.

Would love to hear your comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants