remcoder
Redefine the main init function that handles the full lifecycle of the library
Very useful for https://github.com/remcoder/gcode-preview/issues/208 Could be useful for https://github.com/remcoder/gcode-preview/issues/186
Currently, the init function is just a function that creates an instance of the WebGLPreview class. Since objects added to a WebGL must be properly disposed of and the different components are split out, a main object should be responsible for managing all the moving parts.
I started doing a proof of concept: https://github.com/remcoder/gcode-preview/pull/169/files#diff-0e7851300750764bf1798e6ba4dc51f241c919cb43b45190ccfa0b91f30205cc
In the POC above, I only abstracted some interactions so that the lib users never interact directly with the WebGLPreview instance. Managing the lifecycle of the app was not started, but it's clearly the main advantage of it.
I suggest aiming for 3.0-alpha2
This refactor is another "platform" improvement that should be introduced while the API is still unstable. Then, we'll be able to build the 3.1 features on top of this.
Assuming we still want a setup like you poc'ed in the linked PR, here's a suggestion:
in instead of exposing individual props, like layers, and wiring them up, how about we just expose theWebGLPreview instance. Maybe we should rename WebGLPreview to renderer. Typescripts public/private modifiers will guard access to internal props.
That is a good approach. I did not want to initially because the class was doing way too much. That way, support could be maintained as code was moved.
But the refactor to extract parsing/interpreter/job came first, and the separation of concerns makes sense in terms of public API. Exposing a renderer directly will give much more control! We won't have to protect the internals as much in fear that external callers break how the class is supposed to work.
I'm almost done with the refactor and it's better than I expected. Simpler than my POC since a lot of the required extractions were done. Going to push a draft PR soon.
Quick question:
I added a bunch of getters and setters as legacy support of the original API with deprecation warnings. Planning to remove them in a following release to make the update process easier on alpha version users. Do you think that's too much and it's enough to document the breaking change? The upgrade path is very simple. But it's also easy to make it smooth.
I applied your suggestion to rename WebGLPreview to Renderer. Makes it clearer.
I'm also exploring ways to improve the default values logic to the input type. It's an object with a high number of attributes, that are then used for the renderer's own attributes. Before opening the PR, I'll remove those changes, as they probably belong in a separate PR.
Excited your working on this!
I wouldn't call it too much because it depends on the energy you want to put into it (and the energy you get out of it). I would personally keep it simple for ourselves and I don't consider backwards compatibility a must (we've declared it a new major version for good reason).
So this is really up to you.