Skip to content

Avoid implied color space dependencies for procedural usage#734

Open
aduth wants to merge 1 commit into
color-js:mainfrom
aduth:fix/gamut-implied-oklch-dependency
Open

Avoid implied color space dependencies for procedural usage#734
aduth wants to merge 1 commit into
color-js:mainfrom
aduth:fix/gamut-implied-oklch-dependency

Conversation

@aduth
Copy link
Copy Markdown

@aduth aduth commented May 14, 2026

Updates utility functions to use color space objects directly rather than retrieving the color space by a string lookup, which infers a dependency that those color spaces are already registered.

This improves usability of the procedural API. Currently, if someone opts-in to the procedural API and wants to use these utilities, the consumer has to ensure that the color space is registered, which is not obvious.

Using the color space object directly ensures these functions are independently self-contained and don't have any implied dependencies.

I noticed similar implied dependencies in lighten and darken functions, but given the "alternatives" fallbacks used there, it's less clear to me how best to handle those color spaces. I'm happy to update those functions as well if there's a direction that makes sense.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 14, 2026

Deploy Preview for colorjs ready!

Name Link
🔨 Latest commit 3355ecf
🔍 Latest deploy log https://app.netlify.com/projects/colorjs/deploys/6a05df92b82a9200087bfc3f
😎 Deploy Preview https://deploy-preview-734--colorjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

Comment thread src/toGamut.js
return Math.max(parseFloat(`1e${order - 2}`), 1e-6);
}

const GMAPPRESET = {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally unrelated to this PR, but we shouldn't be having colorspace-specific stuff in a generic function like this. Especially for non-essential color spaces. This should live together with the HCT color space!

@LeaVerou
Copy link
Copy Markdown
Member

I need to look more closely but it seems that the oklch space is only used in the toGamutCSS() path? If so, this makes tree shaking worse, since it essentially turns a peer dependency that is rarely needed into a regular dependency.

It's probably not a huge deal since most people do need oklch, and it's pretty small but thought I'd flag it and see what others think.

That said, I'm not even sure we should have toGamutCSS() in core. CSS itself is moving away from this! We should probably move the whole thing to a plugin…

@aduth
Copy link
Copy Markdown
Author

aduth commented May 15, 2026

If my read of the code is accurate, toGamutCSS would be the default path for all toGamut usage unless method is overridden to something other than 'css' (gamut_mapping defaults to 'css' which toGamut directs to toGamutCSS)?

@lloydk
Copy link
Copy Markdown
Collaborator

lloydk commented May 15, 2026

I need to look more closely but it seems that the oklch space is only used in the toGamutCSS() path? If so, this makes tree shaking worse, since it essentially turns a peer dependency that is rarely needed into a regular dependency.

It's probably not a huge deal since most people do need oklch, and it's pretty small but thought I'd flag it and see what others think.

toGamutRayTrace() also uses oklch.

That said, I'm not even sure we should have toGamutCSS() in core. CSS itself is moving away from this! We should probably move the whole thing to a plugin…

I would like to see some kind of plugin system so that all of the gamut mapping methods aren't in a single file and additional gamut mapping methods could be added e.g. https://bottosson.github.io/posts/gamutclipping/

@lloydk
Copy link
Copy Markdown
Collaborator

lloydk commented May 15, 2026

I haven't looked that closely but couldn't this PR be as simple as:

  • import oklchSpace from "./spaces/oklch.js";
  • delete the calls to ColorSpace.get("oklch")

@facelessuser
Copy link
Copy Markdown
Collaborator

I generally agree that as the gamut mapping options grow, making gamut mapping methods into plugins probably makes more sense, though that is tangential to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants