-
-
Notifications
You must be signed in to change notification settings - Fork 84
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
Tree-shaking ready API #159
Comments
We generally strive to make this modular so that one can import what they need. However, I'm aware modular and tree shakable are not the same thing. If the API was more tree-shakeable, modules that were imported but not used would be eventually pruned. We already pretty much separate functionality into modules: Each color space lives in its own modules and exports its It's a similar story with methods too, and those are a bit trickier because Now, again if my understanding is correct, to make the API tree shakeable, we'd need to eliminate these side effects. This means that importing a color space would go from: import "COLORJS_ROOT/src/spaces/p3.js"; to something like: import ColorSpace from "COLORJS_ROOT/src/space.js";
import p3 from "COLORJS_ROOT/src/spaces/p3.js";
ColorSpace.register(p3); While a bit more verbose, I could maybe stomach that. But something that includes methods, such as e.g. the interpolation module would go from: import "COLORJS_ROOT/src/interpolation.js"; to: import {mix, range, steps} from "COLORJS_ROOT/src/interpolation.js";
Color.mix = mix;
Color.range = range;
Color.steps = steps;
Color.prototype.mix = function(...args) {
return mix(this, ...args);
};
Color.prototype.range = function(...args) {
return range(this, ...args);
};
Color.prototype.steps = function(...args) {
return steps(this, ...args);
}; Which is …quite a lot of manual plumbing 😕 I wonder if there's a way to cater to both use cases somehow. Some way to have the side effects by default and somehow set a flag that each module would read and not perform any side effects if the flag is set. 🤔 |
Do we really need class methods? Extending prototype is not typesafe (easy to forget to import extension and broke your code). |
Are you proposing to have the optional modules be procedural or to not have any instance methods at all? It's an interesting idea, one I need to think about a bit more. Some initial thoughts: For things like the functions in the interpolation module, deltaE, or contrast and things like that, they could totally be procedural. One could even argue that functions that compute something for two or more colors should not be instance methods anyway. There's also a tension between convention and configuration here: more experienced devs need full control, less experienced devs need the ease of just importing one thing and having everything there. It's possible to accommodate both through multiple different bundles, but then the docs become a mess. |
I like the idea to move all methods to functions, so JS bundle will not have any unused code.
What is a problem for new developers with function API? I think that functions even simpler. You do not need to know OOP to use them. |
Ok, I've had an idea so we can have our cake and eat it too:
This way, whoever wants to use Color objects can, and whoever wants to use plain functions also can! I've experimented a bit with this concept locally and it yields a 4x performance increase too (which was my primary motivation for looking into it)! |
Will it promote a worse web performance? What is a rational reason to use instance API in a first place (out of compatibility for old clients)? |
Compatibility is not much of a concern since technically this library is still unreleased 😛 Some people (myself included) consider the OO API nicer. There are many use cases involving color stuff, some very performance sensitive (such as drawing a gradient pixel by pixel) and some less so (such as computing lighter versions of the handful of colors in a palette). Even if the OOP syntax is 4x slower, that means nothing when all you're doing is a few operations (the difference starts becoming noticeable after hundreds of thousands of operations). So for the simpler use cases, I don't see what's wrong with giving people options and letting them pick the API they like best. 😊 |
Just to clarify. For I think OOP has the same runtime performance. This issue is addressing loading performance and my dream to have less JS on modern websites. |
Even if it's not tree-shakeable, it's still modular, as in, you can import only the modules you need. Don't forget that not all users use bundlers.
OOP does not have the same runtime performance at all. I wish it did! When I switched the API locally to use object literals instead of |
Right now library exports one big class with everything inside.
It is bad for website performance because webapp JS bundle will contain all methods of Color.js even if user use only a few of them.
Modern JS bundler has tree-shaking feature. If you will move rare (or all) methods to separated functions with separated export, JS bundler will keep only used functions in the JS bundle.
I am talking about something like:
I understand that this feature requires a full refactoring and will not be implemented soon. Just an idea for better web performance.
Color spaces could be tree-shaked as well. Here is an example from
culori
.The text was updated successfully, but these errors were encountered: