(css): Allow injecting UserTheme and typecheck colors against scale

[hasparus]

This is IMHO the coolest PR I've sent to this repo. An actual feature!

A move towards "TypeScript: Type check property values against theme scales" v1 goal. #832

I belive we can treat Theme UI as an embedded domain-specific language for branded styling.

Reasoning

Imagine you're working with strict designers. They specify what's on-brand and what's not, the list of colors, spacings etc.

We keep these design tokens in our theme scales, but we can still use arbitrary values, merge them to master, deploy and get a dissapointed slack message from a designer. What if we could constain ourselves to predefined values on type level to make sure we don't do this?

Now we can.

I kinda like the idea of the runtime working as it is now and supporting arbitrary values all the time.
They are useful for prototyping, even if we don't want to merge them to master.

If we decide to not build the app on type errors, we effectively make illegal states unrepresentable.

WIP

Any help is welcome! 🙌

• Benchmark TypeScript compilation times in projects using this branch in comparison with current develop.
• I'd like to refactor css/src/types a bit because it turned 800 lines of circular references. Maybe we can colocate types with the runtime code?
  • We need to import more types from Emotion, get rid of the ones that were copied from DefinitelyTyped.
  • Note: Theme depends on variants and scales. Variants depend on ThemeUIStyleObject. ThemeUIStyleObject depends on Theme and "platform" CSS types.
• Describe it in the docs.
• Tests!

---

@nandorojo, will this work with dripsy? I'd guess so, but I only skimmed through your codebase.

---

---

The solution is similar to

DefaultTheme injection in styled-components:
https://styled-components.com/docs/api#create-a-declarations-file
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/styled-components/index.d.ts#L412

And declaring config in Overmind:
https://overmindjs.org/core/typescript#1-declare-module.

---

Closes #1297.

[nandorojo]

Ohh love this. If I understand correctly, I think it would close this dripsy issue? I've been thinking about the right way to do typescript intellisense a lot. This PR seems really elegant, I would definitely implement it.

[hasparus]

Oh! I was looking for that gif!

You previously had it in the readme, am I right?

I think merging this (okay, finishing and merging, there's still some work ahead) closes the issue in Dripsy and the only thing left to do there would be an update of Theme UI version.

https://github.com/nandorojo/dripsy/blob/master/src/css/types.ts#L1-6

We don't need any codegen, the user will just opt-in to strict mode (forced design tokens mode?) with declaration merging.

[nandorojo]

Yup I put it in the readme when I was first dreaming up dripsy and didn't want to forget to work on it. I can't wait to add it back on as a feature, not an idea. Super excited. Much better to avoid code gen.

[hasparus]

Okay, I've noticed a limitation.

Without codegen we can't concatenate strings on type level. What does it mean?

We can't statically know that

{
  colors: {
    gray: ["#eee", "#bbb", "#333", "#111"]
  }
}

results in

gray.0
gray.1
gray.2
gray.3

so nested scales would need codegen.

We could probably do a hack like

type Color = keyof UserTheme['color'] | (string & {})

to allow arbitrary values.
Tbh I'd personally be fine with flat scales and writing gray-1.

[nandorojo]

Does the new TypeScript 4.x tuple approach solve that?

https://devblogs.microsoft.com/typescript/announcing-typescript-4-0-beta/

(I don't know the answer.)

I typically use flat scales anyway, so I'd also be fine with that. I would prefer type consistency than nesting values.

[nandorojo]

@hasparus Hey, just checking to see if you're still planning on working on this. Really excited for it.

Relevant from Material UI: https://material-ui.com/components/about-the-lab/#typescript

[hasparus]

I'm wondering if we should migrate to csstype v3.
https://github.com/frenic/csstype/blob/master/index.d.ts#L18131

It uses ... | (string | {}) in every property that has ... | string in v2, so we could just use the types from there and add the keys from exact theme to them.

@hasparus Hey, just checking to see if you're still planning on working on this. Really excited for it.

@nandorojo

Well, I am, but slowly :( Would you like to take over?

[nandorojo]

@hasparus No worries, I understand. I'm not sure I'm enough of a TS wizard for this one, but I'm happy to try to help...could you describe the steps for adding other scales?

[nandorojo]

@cmaycumber any shot you've done this kind of declaration merging before? I'm going to work off this branch to try to solve nandorojo/dripsy#6, but I haven't done it before. (Not sure if this feature would be useful to your apps or not, but I think it's one of the more exciting things theme-ui/dripsy could add.)

@hasparus would this PR get merged if we made progress on it?

[cmaycumber]

@nandorojo No, I have not. But this feature is super exciting and I'd love to see it in dripsy and theme-ui, I think it would be a great addition and would be definitely relevant in my apps/websites.

On that note, I'd definitely help as much as possible. I'll try to create a fork of this branch and work with it a bit to see if I can help.

[nandorojo]

Awesome, that's great to hear. I'm going to look into @hasparus work on this so far and try to do the same!

[nandorojo]

@hasparus is there a repository you use somewhere that has the JSDoc comments for the CSS properties? For example, where did you get this from:

export interface ColorScaleCSSProperties {
  /**
   * The **`color`** CSS property sets the foreground color value of an element's text and text decorations, and sets the `currentcolor` value. `currentcolor` may be used as an indirect value on _other_ properties and is the default for other color properties, such as `border-color`.
   *
   * **Syntax**: `<color>`
   *
   * **Initial value**: Varies from one browser to another
   *
   * | Chrome | Firefox | Safari |  Edge  |  IE   |
   * | :----: | :-----: | :----: | :----: | :---: |
   * | **1**  |  **1**  | **1**  | **12** | **3** |
   *
   * @see https://developer.mozilla.org/docs/Web/CSS/color
   */
  color?: Color
...
}

I don't see it in this format on mozilla's site. Thanks!

[hasparus]

OMG you guys are picking this up! I'm so grateful. Sorry for slow responses :c

BTW good news: TypeScript 4.1 makes strictly typing nested scales possible.

@hasparus is there a repository you use somewhere that has the JSDoc comments for the CSS properties? For example, where did you get this from...

All of these should already in Theme UI codebase. I think it's originally from CSSType, but there's no way to "inherit" these comments, so we need to copy them if we want them to appear in user tooltips.

[nandorojo]

@hasparus no worries, thanks for the response. It might make sense for one of us to move this into our own PR if you don’t plan on working on it for now.

Would you mind telling me if this is going in the right direction? hasparus#1

Thanks so much for this! Really excited for this feature.

[hasparus]

@nandorojo the Stitches PR I mentioned in Twitter DMs.

stitchesjs/stitches#206

Our code and tests will end up looking pretty similar I suppose.

[nandorojo]

Ah, the GIF in that PR is so satisfying. Thanks for passing this along.

[nandorojo]

I think that makes sense.

btw, ts-toolbelt released a stable dot path notation getter for TS string templates that has higher performance. There’s more discussion towards the end of this issue: millsp/ts-toolbelt#154

I believe it’s F.AutoPath<T, S>

[hasparus]

Okay, I've tested that canary release. It defiintely lacks polish, and I had to work around my circular references.

I had to cast my styles to a simplified type, because the circular reference is inherent to it.

export const makeStyles = (s: ThemeStyles) =>
  s as Record<
    string,
    Record<string, number | false | string | undefined | null> | undefined
  >;

We could split our theme into "specification" and "implementation" parts — scales create define the theme, variants and styles use scales.

So, styles and variants can't be a part of user's exact theme, because this is possible to write.

theme: {
  styles: {
    root: {
      color: theme => theme.styles.root.color
    }
  }
}

---

Additional notes

• If we go further this way, functions from @theme-ui/color should not depend on Theme but on BaseTheme, maybe even on Pick<BaseTheme, 'colors'>. — I'm using alpha to define my colors scale, and since alpha depends on final theme it can't be used to define the final theme.

[nandorojo]

When you say circular references, you're referring specifically to the function getters within the theme, right? (I don't actually use those myself, mostly just in component props, but never in the actual theme.)

[hasparus]

When you say circular references, you're referring specifically to the function getters within the theme, right?

Yes. Functions in the theme get theme as the argument. (I'm wondering if TS would allow a circular reference if we made theme "methods" instead (this: Theme) = StylePropertyValue). Variants, and styles are the consuming part of the theme, scales are the definition part of the theme.

But functional styles aren't the only problem. Take a look at this:

const root: ThemeUICSSObject = {
  backgroundColor: "background" 
}

const theme = makeTheme({
  styles: { root }
})

type ExactTheme = typeof theme;

declare module "@theme-ui/css" {
  interface UserTheme extends ExactTheme {}
}

ThemeUICSSObject depends on the final Theme type, final Theme type is Assign<BaseTheme, UserTheme>, and UserTheme depends on theme.styles.root, so we have a circular.

CodeSandbox with the example: https://codesandbox.io/s/circular-explosions-4re43?file=/src/App.tsx

We could change the API to work around this

function makeScales<TExactScales extends Scales>(scales: TExactScales): TExactScales {
  return scales
}

const scales = makeScales({ /* ... */ })

type MyScales = typeof scales

declare module "theme-ui" {
  export interface UserScales extends MyScales {}
}

And stop caring about keeping styles and variants in Theme type.
We could say "Using functional styles to access theme.styles and variants is discouraged, use sx.variant instead".

[nandorojo]

@hasparus I think this makes sense. The declaration merging is only going to be once you create the app. It’s totally fine to ask people to have a MyScales, MyColors, etc types. They can then feed those into their theme. I don’t think we need declaration merging on the entire theme.

Plus, if we have a makeScales function and such, we can add some validation makeScales<T extends ...>.

[nandorojo]

Basically, I’m thinking the final theme type shouldn’t rely on the user theme. The theme has too many things that the user shouldn’t need to edit and will create circular references. They should just add their own types for colors, scales, etc, and the final theme object can pull all of those in.

[nandorojo]

Also, since the “theme” itself doesn’t need intellisense, and only components do, maybe the user types should just get mapped onto the sx prop, but get ignored when writing variants, etc on your actual theme if needed. It seems like a big step up to have strict types on the sx prop, and at this point I’d love if there’s a way to get this PR to land :)

[nandorojo]

In case it's of interest, I implemented this from scratch for Dripsy v3: nandorojo/dripsy#124

[yerffejytnac]

Any updates on this? Haven’t seen any commits in a while and was curious if it’s still a feature being actively developed.

🙏

[hasparus]

@yerffejytnac I'm gonna salvage some parts of this PR, but after testing a prerelease we discovered this is not exactly the public API we'd like to have, as it can be a bit of a footgun.

I'm recently using Theme UI a lot more than I used it in the past years, so while I won't promise you an expected release date, it's quite likely I'll need this or an alternative feature will get implemented soon :)

[MuhammadJamaluddin]

Any updates on this?