Adds theme keypaths to our sx prop type for better autocomplete #1544
Conversation
🦋 Changeset detectedLatest commit: 82a4a68 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
jfuchs
force-pushed
the
jfuchs/handcrafted-theme-type
branch
from
fd8ec49
to
5de709f
Compare
src/theme.ts
Outdated
| subtle: string | ||
| } | ||
|
|
||
| // Only meant for Primer components |
There was a problem hiding this comment.
I'm not sure if we want autocomplete for primer.* variables. Let's leave them out for now.
src/theme.ts
Outdated
| export type ThemeShadowPaths = KeyPaths<ThemeShadows> | ||
|
|
||
| // Produces a union of dot-delimited keypaths to the string values in a nested object: | ||
| type KeyPaths<O extends {}> = { |
There was a problem hiding this comment.
Should this live in src/utils/types/?
jfuchs
force-pushed
the
jfuchs/handcrafted-theme-type
branch
from
5de709f
to
3095551
Compare
jfuchs
force-pushed
the
jfuchs/handcrafted-theme-type
branch
from
3095551
to
bacbdf0
Compare
.changeset/silly-coins-sit.md
Outdated
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| '@primer/components': patch | |||
There was a problem hiding this comment.
I'm on the fence about whether this is a minor or patch release. Do we consider the lack of types a bug?
There was a problem hiding this comment.
Hmm, good question. Maybe it's a minor? It's sort of a new feature, but also the API hasn't really changed at all? Even the types should be functionally equivalent — anything that passed typechecking before passes now, and anything that didn't pass before fails now.
There was a problem hiding this comment.
(TLDR: no strong feelings, happy to switch to minor if you think that's more appropriate)
| } | ||
| } | ||
|
|
||
| export function generatesKeyPathsFromObject(x: KeyPaths<NestedObject>): 'a' | 'b.b1' | 'b.b2' { |
There was a problem hiding this comment.
Thanks again for setting these up!
jfuchs
changed the title
|
Exciting!!! 🎉 |
This replaces the type currently used for the
sxprop with one that explicitly includes all key paths into our theme for color and shadow values. This does not yet add better typing to styled system props.Partially addresses https://github.com/github/primer/issues/389
Open questions
ColorPropsandShadowPropsobjects an acceptable proxy for getting a list of keys intoSystemCssPropertiesobjects that have color and shadow variables? In theory styled system prop names could diverge fromSystemCssPropertiesproperties' names over time.Screenshots
CleanShot.2021-10-27.at.15.45.40.mp4
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.