Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stricter type support #72

Closed
nandorojo opened this issue Jan 26, 2021 · 5 comments
Closed

Add stricter type support #72

nandorojo opened this issue Jan 26, 2021 · 5 comments

Comments

@nandorojo
Copy link
Owner

nandorojo commented Jan 26, 2021

I'm still waiting on system-ui/theme-ui#1090 to merge, and it seems to be getting hung up.

I think it might be time to upstream theme-ui on types for certain things.

I have 2 goals:

  • Provide stricter types that match Dripsy, such as Theme['customFonts']
  • Let users use declaration merging to type useDripsyTheme.
    • I'd like to achieve the same with the sx prop, but it's too complicated so I'll wait for the PR above.

1) Edit the default Theme type, name it DripsyTheme

For starters, I'd like to override the default Theme type exported from theme-ui, which does not include dripsy's customFonts key in the theme.

As we can see here in this app by @byCedric, customFonts isn't included in the type, so you need to either ignore Theme, or make it a separate type. This doesn't seem ideal.

// types.ts
import type { Theme } from 'theme-ui'

export interface BaseDripsyTheme extends Theme { customFonts: { ... } }

Custom Font type implementation

Since Dripsy has strict requirements for custom fonts, it would be nice to enforce them with types too:

export interface BaseDripsyTheme<FontFamilyKeys extends string> extends Theme {
  customFonts?: { [key in FontFamilyKeys]?: Record<string, string> }
  fonts?: {
    [key: string]: FontFamilyKeys | undefined
    ['root']?: FontFamilyKeys
  }
}

Unfortunately, it seems like this code sample doesn't actually work as intended, so I'd love suggestions on that.

2) Provide strict types for useDripsyTheme

As it stands, useDripsyTheme has relatively weak typings. At the very least, I'd like it to piggy back off of 1) above, like so:

// use-dripsy-theme
import { BaseDripsyTheme } from './types.ts'

export default function useDripsyTheme() {
  return (useThemeUI() as any) as Omit<ReturnType<typeof useThemeUi>, 'theme'> & { theme: BaseDripsyTheme }
}

Here, we at least get the customFonts field with useDripsyTheme.

However, I think we could take it a step further with declaration merging. This is what system-ui/theme-ui#1090 intends to achieve for the sx prop. In the meantime, I think we can hack our way around it for useDripsyTheme.

Strict types implementation

Imagine we have this theme:

const theme = {
  colors: {
    primary: 'red',
    'primary-dark': 'darkred'
  }
}

A fair expectation from the user should be this:

const { colors } = useDripsyTheme().theme

const textColor = colors.primary // red

I think it could be achieved with this kind of solution:

Step 1: Dripsy creates a makeTheme function:

An example looks like this from theme-ui's upcoming PR.

// this is from #1 up above
import { DripsyTheme } from './types'

export const makeTheme = <T extends DripsyTheme>(t: T) => t

Step 2: Dripsy exports CustomDripsyTheme

Theme-ui plans to call this UserTheme, so once they do, we can merge them somehow, but this way we don't collide.

/**
 * Can be augmented by users to inject their exact theme into Dripsy types.
 */
export interface CustomDripsyTheme {}

Step 3: Users create their theme with makeTheme:

const myTheme = makeTheme({
  colors: {
    primary: 'red',
    'primary-dark': 'darkred'
  }
})

type MyTheme = typeof myTheme
declare module 'dripsy' {
  // this overrides the empty CustomDripsyTheme that Dripsy exports
  export interface CustomDripsyTheme extends MyTheme {}
}

Step 4. Make FinalDripsyTheme

We can combine the original BaseDripsyTheme with CustomDripsyTheme:

import type { Theme } from 'theme-ui'

export interface BaseDripsyTheme extends Theme { customFonts: { ... } }

export type Assign<T, U> = {
  [P in keyof (T & U)]: P extends keyof U
    ? U[P]
    : P extends keyof T
    ? T[P]
    : never
}

export interface DripsyTheme extends Assign<BaseDripsyTheme, CustomDripsyTheme> {}

// For backwards-compatibility, name Theme too
export type Theme = DripsyTheme

The code above is taken from theme-ui here.

Now that we have this DripsyTheme created, we can use that in useDripsyTheme:

// use-dripsy-theme
- import { BaseDripsyTheme } from './types.ts'
+import { DripsyTheme } from './types.ts'
// use-dripsy-theme

export default function useDripsyTheme() {
  return (useThemeUI() as any) as Omit<ReturnType<typeof useThemeUi>, 'theme'> & { theme: DripsyTheme }
}
@nandorojo
Copy link
Owner Author

This should work in a @dripsy/theme package one @cmaycumber's #74 gets merged, will try to get to merging that when I can.

@JackCA
Copy link

JackCA commented Apr 28, 2021

@nandorojo it looks like #74, does that help push this along at all?

PS: You've been doing really great work on the recent releases!

@nandorojo
Copy link
Owner Author

Yeah, I haven't really had time to work on this. At the moment, I'm just using this in my app:

import { useDripsyTheme } from 'dripsy'
import type { DripsyTheme } from './index'

export default function useTheme() {
  return (useDripsyTheme().theme as any) as DripsyTheme
}

@nandorojo
Copy link
Owner Author

This will be added in v3. See #124 🥳

@nandorojo
Copy link
Owner Author

Coming very soon...

Code.-.hook.tsx.dripsy.mp4

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

No branches or pull requests

2 participants