useColorSchemeVar should provide explicit color assistance/types and/or primer reexports primitives

[mattcosta7]

Describe the bug

In order to use things like scale values from primer/primitives, primer/react recommends using https://primer.style/react/theming#creating-local-color-scheme-variables

primer/react doesn't explicitly export the colors, or provide the primitive colors on the theme to access them from, which means that this import needs to either by an indirect import (primer/primitives is a dependency of primer/react), but different versions can be a dependnecy on other code. for instance, a component could rely on an older version of primitives, or even the eslint plugin can rely on a different version of primer/primitives - so which version is actually used depends on the bundling algo.

Tools that try to prevent access to non-explicitly depended on packages also may get angry from the indirect imports

The other option is to manually import another version of primer/primitives, which might not conform well to implicitly used version.

I'd suggest that useColorSchemeVar potentially allow for access to the full color set, as well as types that better align with the possible color schemes, to help ensure that generated colors are valid everywhere - even if just as suggestions

Primer could also just re-export the primitives consumers might access as a way to tie these together and import everything from primer

[siddharthkp]

Yes to everything you said.

In additional to that, would a runtime check for primitives version in primer/react help find any problems during development

so which version is actually used depends on the bundling algo

Sorry, just to understand this more, primer/react has a direct dependency on primer/primitives: package.json#L83. That should make sure a component always gets the correct primitives version, right?

---

Bonus: It would make sense that all packages in primer ecosystem resolve to the same version to prevent any duplicates in their latest version:

- primer/primitives = 7.8.3

- primer/react → @primer/primitives 7.6.0 🛑 bad! it's locked to 7.6.0 instead of ^7.6.0
- primer/css → @primer/primitives ^7.8.3 ✅ good!
- primer/view_components → @primer/css ^20.0.0 → @primer/primitives ^7.7.0 ✅ good!
- primer/eslint-plugin-primer-react → @primer/primitives >=4.6.2 (peerDependencies) ✅ good!
- primer/stylelint-config, only dev dependencies ✅ good!

safe to ignore for now:
- primer/react-brand → @primer/[email protected] (devDependency)

[mattcosta7]

Yep - @primer/react means components will always get the correct version, but if a user was to install @primer/primitives to work with the guidance on useColorSchemeVar now, they might install a version that isn't the same as the one primer/react depends on, which I think might be where the gremlins could show up (either having 2 versions installed, or resolving to 1 version at build time, and which one!?)

[mattcosta7]

@siddharthkp

Would something like

useColorSchemeVars(colors => [partial<Record<color schemes, color>, fallback]> work here, to at least avoid leaking the primitives dependency into user code?

Not sure how that might Play with consumer themes outside of primitives default, since I'm not sure whether there's access to this from the theme provider currently, but something along that pattern seems like it might help here?

I don't know that we'd need to return a tuple with a fallback, if the assumption is that consumers should be complete here instead - just having that to maintain parity with the current impl

That also might be fine to not do anything extra, assuming this internal colors is just the primitives one, and users can always use the plain object API directly if they have a different primitive theme?

As an aside - is there terminology for the theme as accessible from useTheme and the ones passed to themeprovider (since one has color scheme merged and one has all color schemes) it feels weird calling both theme!

[siddharthkp]

useColorSchemeVars(colors => [partial<Record<color schemes, color>, fallback]> work here, to at least avoid leaking the primitives dependency into user code?

Yep, that's exactly what I was thinking of.

Either that or exporting primitives from primer/react:

- import {useColorSchemeVar } from '@primer/react'
- import {colors} from '@primer/primitives'

+ import {useColorSchemeVar, primitives} from '@primer/react'

[lesliecdubs]

Thanks for discussion here! I'm going to move this to our unprioritized backlog so we can consider type safety for primitives in our next round of Primer planning. To be continued!

[github-actions[bot]]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[mattcosta7]

not stale

[github-actions[bot]]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[mattcosta7]

not stale

[github-actions[bot]]

Hi! This issue has been marked as stale because it has been open with no activity for 180 days. You can comment on the issue or remove the stale label to keep it open. If you do nothing, this issue will be closed in 7 days.

[mattcosta7]

not stale