color-js / color.js

Color conversion & manipulation library by the editors of the CSS Color specifications
https://colorjs.io
MIT License
1.89k stars 82 forks source link

Tree-shaking ready API #159

Closed ai closed 2 years ago

ai commented 2 years ago

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:

import { contrast } from 'colorjs.io'

contrast(color)

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.

LeaVerou commented 2 years ago

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 ColorSpace object as a default export. However, each color space also registers themselves onto ColorSpace.registry, which I guess means they would not be tree shaken even if not actually used.

It's a similar story with methods too, and those are a bit trickier because Color is structured as a class. A pattern we've followed recently for non-essential stuff is to have the module add its functions onto Color and Color.prototype as well as exporting them to be used in the way you describe. For an example of this see interpolation.js. Contrast is definitely a good candidate for this sort of thing, we just hadn't gotten around to it yet.

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. πŸ€”

ai commented 2 years ago

date-fns way looks better for me and very popular in modern Web, rather than adding methods to Color prototype.

Do we really need class methods?

Extending prototype is not typesafe (easy to forget to import extension and broke your code).

LeaVerou commented 2 years ago

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. For other things that are a) essential and b) operate on the color instance itself like toString() or toGamut(), I'm inclined to think they make sense as instance methods.

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.

ai commented 2 years ago

Are you proposing to have the optional modules be procedural or to not have any instance methods at all?

I like the idea to move all methods to functions, so JS bundle will not have any unused code.

less experienced devs need the ease of just importing one thing and having everything there.

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.

LeaVerou commented 2 years ago

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)!

ai commented 2 years ago

This way, whoever wants to use Color objects can, and whoever wants to use plain functions also can!

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)?

LeaVerou commented 2 years ago

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. 😊

ai commented 2 years ago

Just to clarify. For bad web perfomance I mean that OOP is forcing to bloat JS bundle size by adding unnecessary functions to website JS file.

I think OOP has the same runtime performance. This issue is addressing loading performance and my dream to have less JS on modern websites.

LeaVerou commented 2 years ago

I mean that OOP is forcing to bloat JS bundle size by adding unnecessary functions to website JS file.

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.

I think OOP has the same runtime performance.

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 Color instances the performance increase was huge (my benchmark went from 1400ms to 750ms!). There was literally no other change, just not creating Color instances and passing object literals around instead was that much faster.