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

fix: add types for ElementRef support #237

Merged
merged 3 commits into from
Sep 14, 2022
Merged

fix: add types for ElementRef support #237

merged 3 commits into from
Sep 14, 2022

Conversation

jjenzz
Copy link
Contributor

@jjenzz jjenzz commented Sep 14, 2022

Note: Easier to review with hidden whitespace changes

Closes #225

@vercel
Copy link

vercel bot commented Sep 14, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
dripsy ❌ Failed (Inspect) Sep 14, 2022 at 8:46PM (UTC)

Comment on lines -22 to -25
P extends Omit<Props, 'variant' | 'variants'> = Omit<
Props,
'variant' | 'variants'
>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nandorojo
Copy link
Owner

This is 🔥🔥🔥

I never knew how to get inferred refs to get to work. I'd love to get this to work for moti too. I'll try to copy this over there.

@nandorojo
Copy link
Owner

One thing – I'm getting this error when I run yarn (which triggers yarn prepare):

src/css/create-themed-component.tsx(48,52): error TS2345: Argument of type 'PropsWithChildren<StyledProps<ThemeKey> & Props>' is not assignable to parameter of type 'ExtraProps'.
  'ExtraProps' could be instantiated with an arbitrary type which could be unrelated to 'PropsWithChildren<StyledProps<ThemeKey> & Props>'.

@nandorojo
Copy link
Owner

I added a commit to ignore it – I think it's fine since it's just an implementation detail.

@nandorojo
Copy link
Owner

Let me know if it's good to go, and I'll merge + release.

@jjenzz
Copy link
Contributor Author

jjenzz commented Sep 14, 2022

i'll take a quick look at that issue, i think i know the cause...

@jjenzz
Copy link
Contributor Author

jjenzz commented Sep 14, 2022

@nandorojo all should be good to go now :)

@nandorojo nandorojo merged commit 358ea61 into nandorojo:master Sep 14, 2022
@nandorojo
Copy link
Owner

Published in 3.8.0!

@jjenzz jjenzz deleted the fix-element-ref branch September 14, 2022 21:03
@jjenzz
Copy link
Contributor Author

jjenzz commented Sep 14, 2022

@nandorojo hmmm, i've pulled the latest version and for some reason the following code is fine in this project but errors in mine. does this work for you?

interface CircleProps {
  size?: number
  color?: 'red' | 'green'
}

const Circle = styled(View)<CircleProps>((props) => ({
  width: props.size || 56,
  height: props.size || 56,
  backgroundColor: props.color ?? 'transparent',
  borderRadius: 9999,
  justifyContent: 'center',
  alignItems: 'center',
  overflow: 'hidden',
}))

export const Foo = () => {
  return (
    <Circle
      size={20}
      sx={{
        backgroundColor: 'crimson',
        position: 'absolute',
        right: -6,
        bottom: 2,
      }}
    />
  )
}

@jjenzz jjenzz mentioned this pull request Sep 14, 2022
@nandorojo
Copy link
Owner

Oh, it's because the first generic of styled became C, when it used to be Props, right?

@nandorojo
Copy link
Owner

const Circle = styled(View)((props: CircleProps) => ({

This fixes it, but it still keeps a breaking change.

@nandorojo
Copy link
Owner

nandorojo commented Sep 14, 2022

I think this should fix it:

export function styled<
  Props,
  C extends ComponentType<any>,
  ThemeKey extends keyof DripsyFinalTheme = keyof DripsyFinalTheme
>(
  Component: C,
  {
    themeKey,
    defaultVariant,
  }: Pick<ThemedOptions<any, ThemeKey>, 'themeKey' | 'defaultVariant'> = {}
) {
  return function dripsyFactory<Extra>(
    defaultStyle?: ThemedOptions<Extra & Props, ThemeKey>['defaultStyle']
  ) {
    return createThemedComponent<C, Extra & Props, ThemeKey>(Component, {
      defaultVariant,
      themeKey,
      defaultStyle,
    })
  }
}

This way, we can still explicitly pass a props generic to styled()

@nandorojo
Copy link
Owner

Hm, that actually did not fix it for me. I'll have to keep looking.

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

Successfully merging this pull request may close these issues.

Can't get ElementRef type from styled comp
2 participants