weixin_39864738
weixin_39864738
2021-01-08 13:06

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

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. https://github.com/system-ui/theme-ui/issues/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! 🙌

  • [x] Needs support for all scales, I just did color.
  • [x] colors
  • [x] opacities
  • [x] space
  • [x] fonts
  • [x] fontSizes
  • [x] fontWeights
  • [x] lineHeights
  • [x] letterSpacings
  • [x] borders
  • [x] borderWidths
  • [x] borderStyles
  • [x] radii
  • [x] shadows
  • [x] zIndices
  • [x] size
  • [x] 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!

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

image

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 https://github.com/system-ui/theme-ui/issues/1297.

该提问来源于开源项目:system-ui/theme-ui

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

40条回答

  • weixin_39542936 weixin_39542936 4月前

    This is all looking good.

    The only thing I'm left wondering is how to extend the sx prop. While dripsy follows the theme-ui spec, there are a few styles that CSS doesn't support that React Native (or React Native for Web) does.

    For example, React Native for Web supports animationKeyframes in the style prop.

    Similarly, React Native's transform looks like this:

    jsx
    <view sx="{{" transform: translatey:></view>
    

    I added transform to dripsy (https://github.com/nandorojo/dripsy/issues/39) to fix that, but it isn't typed. Would this raise TS errors with the sx prop proposed in this PR? If so, is there an easy way to extend the sx prop?

    As it stands, it looks like any JSX element in a .tsx file thinks it can accept the sx prop. This is nice, but I'd like to be able to extend/edit that sx interface if possible.

    I'm a bit worried that we need more tests and better infra (like preview releases on CI) to start adding features like this smoothly.

    I could try to test this in my app if we could get it published to a `` tag or something like that.

    There is a similar situation with box-shadows in Dripsy, since RN doesn't use a string to define a shadow the way CSS does (see https://github.com/nandorojo/dripsy/issues/17#issuecomment-697538485).

    image

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    If so, is there an easy way to extend the sx prop?

    I think we can use declaration merging for that. I'll try that and send you a snippet.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    Awesome, thanks.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    is there anything we're missing here to move towards a merge?

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    has been incredibly helpful here with giving insight on generating template literal strings from an object type. It seems like we could use his solution for adding strict nested fields with TS 4.1.

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    would you have a moment to give this PR another look? you promised me code review.

    There should be no breaking changes at runtime, and I think there's also no breaking changes on type level.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    Sure thing. CC'ing too since he mentioned this would be useful in his app.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    is there a way that I can install this into my app to stress test?

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    The only thing I'm left wondering is how to extend the sx prop. While dripsy follows the theme-ui spec, there are a few styles that CSS doesn't support that React Native (or React Native for Web) does.

    Wanted to check in about that comment.

    mentioned we could probably extend the sx prop with declaration merging. Not sure if that's beyond the scope of this PR or not. Luckily, the types in the code base are getting a lot cleaner, so this seems like it won't be too tough to implement.

    Until the sx prop has declaration merging, I'm thinking I can just extend the SxProp (I forget what its replacement is called) and replace a few fields to fit the React Native styles. I'd have to copy some TS types over from theme-ui, though, to support scales and such.

    点赞 评论 复制链接分享
  • weixin_39581995 weixin_39581995 4月前

    is there a way that I can install this into my app to stress test?

    I recommend using yarn link to try it out! You have to be on this branch, run yarn/yarn prepare then cd packages/theme-ui && yarn link, then in your project yarn link theme-ui.

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    I had to update to next version of TypeScript to work around a stack overflow on template literal types (https://github.com/microsoft/TypeScript/issues/41651), and now microbundle won't build our code :D I think we have to pause this PR for a couple of days, because we're ahead of build tooling.

    
    lerna ERR! yarn run prepare stderr:
    (rpt2 plugin) Error: /home/runner/work/theme-ui/theme-ui/packages/css/src/scales/borders.ts(3,15): semantic error TS2305: Module '"./scales-utility-types"' has no exported member 'ScaleProperty'.
    Error: /home/runner/work/theme-ui/theme-ui/packages/css/src/scales/borders.ts(3,15): semantic error TS2305: Module '"./scales-utility-types"' has no exported member 'ScaleProperty'.
    
    点赞 评论 复制链接分享
  • weixin_39975810 weixin_39975810 4月前

    btw, as an alternative to microbundle for a monorepo setup, I highly recommend checking out https://github.com/preconstruct/preconstruct -- which is a system created by the guys behind emotion. I've recently adopted it in my own projects and fell in love.

    点赞 评论 复制链接分享
  • weixin_39581995 weixin_39581995 4月前

    Thanks! That’s happening in #1204 :)

    点赞 评论 复制链接分享
  • weixin_39697096 weixin_39697096 4月前

    This pull request is being automatically deployed with Vercel (learn more).
    To see the status of your deployment, click below or on the icon next to each commit.

    🔍 Inspect: https://vercel.com/systemui/theme-ui/1immwg7aa
    ✅ Preview: Failed

    [Deployment for 6f94730 failed]

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    which version of TS fixed the stack overflow bug for you?

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    typescript.2.0-dev.20201211 includes PR https://github.com/microsoft/TypeScript/pull/41693

    I'm having a different compilation problem on this one, though. I'm hoping I can solve it today after work.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    I'd also like to get deep paths in scales to make the "strict mode" compatible with all themes.

    Agreed. I think something like this would be good:

    Screen Shot 2020-10-16 at 4 47 42 PM

    Here's a video of it.

    Code sample here.

    (credit)

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    The scales are all complete. Mind looking over them?

    TSDoc comments aren't added yet. Figured it'd be easier to look this over without them. If anyone would be interested in adding comments so this can get merged, I'd appreciate it.

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    The scales are all complete. Mind looking over them?

    Getting right to it today / tomorrow morning.

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    I'm annotating the scales with Record<keyof ScaleProperties, 'scaleName'> to ensure we don't miss anything.

    tsx
    const sizes: Record<keyof sizescssproperties> = {}
    </keyof>
    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    Are we good to check those off in your original post?

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    yup, I think so.

    I’m pretty excited about how it's starting to look.

    I'd also like to get deep paths in scales to make the "strict mode" compatible with all themes.

    I'm a bit worried that we need more tests and better infra (like preview releases on CI) to start adding features like this smoothly.

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    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.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    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.

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    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

    ts
    {
      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

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

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

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    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.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    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

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    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.

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

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

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    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?

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    any shot you've done this kind of declaration merging before? I'm going to work off this branch to try to solve https://github.com/nandorojo/dripsy/issues/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.)

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

    点赞 评论 复制链接分享
  • weixin_39609483 weixin_39609483 4月前

    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.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

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

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

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

    ts
    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** |
       *
       *  https://developer.mozilla.org/docs/Web/CSS/color
       */
      color?: Color
    ...
    }
    </color>

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

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    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.

    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.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    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? https://github.com/hasparus/theme-ui/pull/1

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

    点赞 评论 复制链接分享
  • weixin_39864738 weixin_39864738 4月前

    the Stitches PR I mentioned in Twitter DMs.

    https://github.com/modulz/stitches/pull/206

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

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

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

    点赞 评论 复制链接分享
  • weixin_39690105 weixin_39690105 4月前

    good stuff everyone. so excited for this!

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    I should be able to finish the scales this weekend.

    点赞 评论 复制链接分享
  • weixin_39542936 weixin_39542936 4月前

    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.

    点赞 评论 复制链接分享

相关推荐