hex ignores alpha #125

Thoughts on throwing a warning when calling hex() on color with an alpha channel instead of ignoring it?

Err, why? All colors have an alpha channel.

This isn't an "exists" problem, it's a format problem. There are an equally wide variety of cases where ARGB and RGBA are used.

I agree that it's a format problem, no doubt.

Even though all colors have an alpha channel, what I meant here is to throw a warning (console.warn) when the alpha channel is something less than 1.

Why? Because this library is so well built that it gets too easy to chain through multiple colors and not knowing that one of them had a .fade() applied to it. Therefore, using .hex() on it will just ignore the alpha channel and leading to misleading colors at runtime. Adding a warning there when calling .hex() on color with an alpha channel less than 1 could significantly improve the developer experience by telling us to use .hsl() instead in order to have a hsla which supports the alpha channel, rgba, or ahex/hexa once they are supported.

We could also mention this issue in the warning, cause it's misleading when .hsl() is returning a hsla but .hex() ignores the alpha channel.

I'll be happy to submit a PR if you think it will be great.

No I don't, sorry. console.warn is not platform agnostic and introduces a side effect. There's no guarantee it'll be seen. Plus, I'm against warnings entirely; either it's an error or it's not. Lastly, the alpha check results in another branch which, in tight loops, will hurt performance.

Further, discarding alpha on .hex() is a completely valid usecase.

I've already approved #158. Feel free to create a new version as the author has decided they no longer want to contribute their patches.

However, under no circumstances will I be changing the behavior of .hex() nor will I introduce a conditional branch or a console warn.

The warning could be wrapped inside a development check or something like that as we probably don't want those warnings in production anyway. The alpha check would only happen in that development branch, so there's no performance penalty in production.

discarding alpha on .hex() is a completely valid usecase.

Sure thing, but surprises and inconsistencies in the API are not.

The ideal scenario is probably to introduce a breaking change in order to throw an error when trying to use .hex() with a non-pristine alpha channel, and allowing a safe conversion by passing a parameter to .hex(). That way, we would be able to gracefully downgrade it.

But hey, I guess we'll be doing our own wrapper/patch-package. 🤷‍♂️

But hey, I guess we'll be doing our own wrapper/patch-package.

Sure thing, be my guest. Nobody has had issues with this until now, and proper documentation and alternatives can remedy this if need be.

A stdio side effect inside an algorithmic library is never a solution. An added branch because people choose not to read the documentation or test out the dependencies they bring into their projects is not reasonable.

If you want to add unreasonable changes in your own fork, that's fine. I will not be able to provide support for it and I will not guarantee any compatibilities with it. There has already been a proposed solution, ripe for a PR.

I thought the other PR was a dead end state because you didn’t want to add new APIs into it until you figured out a way to refactor it properly?

If you’re really open to receiving a PR, reviewing it, and getting it merged, I would be happy to send it with an improved docs. Having the possibility to chose between hex, hexa, and ahex will be a great step.

I never said it was a dead-end. OP just closed it. :) Feel free to make a new PR.

Sure thing! I’ll make sure to add test coverage. Anything else I should keep in mind while working on this aside of performance?

As long as tests pass, should be okay for the review. :)

@Qix-

Uhm, what format problems are we talking about here? The RGBA format is universally supported and as such, this library really should include the alpha channel.

It's also the only format documented here on MDN:

The only place I can find ARGB mentioned in relation to browsers, is in articles talking about some ancient DXImageTransform hacks in IE7-8. That browser has been dead and gone for a looong time now, and really shouldn't hold this library back from adhering to the standard.

Therefore, please update the hex() method to return RGBA if the alpha channel is less than 1. Having to write my own wrapper around this today is, with all due respect, absolutely ridiculous - and it's sad too, because the rest of this library is, as others have mentioned too, very nice to work with :-)

@thomas-darling color is not only used in the browser. So no, it's not as straightforward. Also please tone it down on the snark. I won't tolerate that here.

Well, actually, you were the one referring to some browsers being the problem :)

... because some browsers support ARGB and others support RGBA.

Regardless, I still can't think of any place in today's JS ecosystem where ARGB would be used today...

And regarding that snark you refer to - I do respect the great work you did here, as I already stated before, but you also have to respect that people might find it a little bit ridiculous, that this pretty significant limitation is still not documented anywhere, several years after the issue was first reported. Making a decision to break a standard is one thing, leaving it up to thousands of developers to discover that when their code breaks is another - I had bugs in production and wasted time because of this. You can't expect me to be all positive about that.

Regardless, I still can't think of any place in today's JS ecosystem where ARGB would be used today...

WebGL contexts.

And like I said, color is used in a lot of places.

but you also have to respect that people might find it a little bit ridiculous

Sorry, but I don't "have to" do anything. 🙂

Making a decision to break a standard is one thing

Which standard have I broken?

leaving it up to thousands of developers to discover that when their code breaks is another

Where are these "thousands" of otherwise silent developers facing swaths of bugs because rgba/argb hasn't been added?

I had bugs in production and wasted time because of this. You can't expect me to be all positive about that.

That isn't my problem. Thousands of people use this module without any issue. If you're not testing modules before you use them, sounds like a "you" problem, not a "thousands of other developers" problem.

Closing this as I'm tired of the entitled demands and shitty attitudes here.

Submit a PR to replace the other one if you want some movement on this issue. Sure beats wasting my time arguing with you any further.