fix exported types #239

jjenzz · 2022-09-14T21:27:24Z

Fixes issue mentioned here #237 (comment)

The styled.d.ts definition was trying to manually construct the Props type and looks like it was getting it wrong somewhere. Exporting the type will ensure it uses import("./create-themed-component").Props correctly instead.

DIFF

export declare function styled<
  C extends ComponentType<any>,
  ThemeKey extends keyof DripsyFinalTheme = keyof DripsyFinalTheme
>(
  Component: C,
  {
    themeKey,
    defaultVariant,
  }?: Pick<ThemedOptions<any, ThemeKey>, 'themeKey' | 'defaultVariant'>
): <Extra>(
  defaultStyle?:
    | import('./types').Sx
    | ((props: Extra) => import('./types').Sx)
    | undefined
) => import('react').ForwardRefExoticComponent<
  import('react').PropsWithoutRef<
    import('react').PropsWithChildren<
-      C extends ComponentType<infer BaseProps>
-        ? Omit<
-            {
-              [P in Exclude<
-                keyof BaseProps,
-                'variant' | 'variants'
-              >]: BaseProps[P]
-            },
-            keyof import('./types').StyledProps<ThemeKey_1> | keyof Extra
-          > &
-            Extra &
-            import('./types').StyledProps<ThemeKey>
-        : never
+      import('./create-themed-component').Props<C, Extra, ThemeKey>
    >
  > &
    import('react').RefAttributes<import('react').ElementRef<C>>
 >

vercel · 2022-09-14T21:27:28Z

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

Name	Status	Preview	Updated
dripsy	❌ Failed (Inspect)		Sep 15, 2022 at 2:22PM (UTC)

nandorojo · 2022-09-14T21:57:30Z

Hmm but this still doesn't allow the styled<{ myProp: boolean }>() syntax, right?

jjenzz · 2022-09-14T22:25:51Z

oh, i didn't realise that was an intended api 😬 my bad.

what is the expectation if someone does styled<{ test: 'foo' }>(View)<{ test: 'bar' }>(); ?

jjenzz · 2022-09-14T23:16:24Z

i'll have to take a look another time as it is super late here now, sorry.

unfortunately, the C generic would need to be optional for that API to work and i don't know if it is possible to infer C if a type is passed for Props. doing so will cause C to fall back to its default value (instead of inference): microsoft/TypeScript#43119

// would infer correctly
const a = styled(Text)()

// inference would break because `C` would fallback to its default value (likely `ComponentType<any>`)
const b = styled<{ prop?: boolean }>(Text)();

my solution relies on being able to infer the Text component type here in order to get its ElementRef and props 😞

the best solution for now might be to revert my change. or would you consider deprecating the broken api?

jjenzz · 2022-09-15T08:09:22Z

@nandorojo funny what a bit of sleep can do.

this should all be working now assuming the following tests look okay to you? note that when using styled<Props>(Comp)() api you must include Comp props as part of the Props generic (matches behavior in v3.7.4).

export const ExtendedCompWithExtraProps = styled<
  React.ComponentProps<typeof KeyboardAvoidingView> & { custom?: boolean }
>(KeyboardAvoidingView)({})

/* @ts-expect-error behavior cannot be 'test' */
export const ExtendE = () => <ExtendedCompWithExtraProps behavior="test" />
/* should not error */
export const ExtendF = () => <ExtendedCompWithExtraProps behavior="padding" />
/* @ts-expect-error custom cannot be string */
export const ExtendG = () => <ExtendedCompWithExtraProps custom="test" />
/* should not error */
export const ExtendH = () => <ExtendedCompWithExtraProps custom={false} />

jjenzz · 2022-09-15T08:19:22Z

examples/example/App.tsx

+export const ExtendedComp = styled(KeyboardAvoidingView)({})
+
+export const ExtendedCompWithExtraProps = styled<
+  React.ComponentProps<typeof KeyboardAvoidingView> & { custom?: boolean }
+>(KeyboardAvoidingView)({})
+
+export const ExtendedCompWithExtraProps2 = styled(KeyboardAvoidingView)<
+  React.ComponentProps<typeof KeyboardAvoidingView> & { custom?: boolean }
+>({})
+
+/* @ts-expect-error behavior cannot be 'test' */
+export const ExtendA = () => <ExtendedComp behavior="test" />
+/* should not error */
+export const ExtendB = () => <ExtendedComp behavior="padding" />
+/* @ts-expect-error custom does not exist */
+export const ExtendC = () => <ExtendedComp custom="test" />
+/* @ts-expect-error behavior cannot be 'test' */
+export const ExtendE = () => <ExtendedCompWithExtraProps behavior="test" />
+/* should not error */
+export const ExtendF = () => <ExtendedCompWithExtraProps behavior="padding" />
+/* @ts-expect-error custom cannot be string */
+export const ExtendG = () => <ExtendedCompWithExtraProps custom="test" />
+/* should not error */
+export const ExtendH = () => <ExtendedCompWithExtraProps custom={false} />
+/* @ts-expect-error behavior cannot be 'test' */
+export const ExtendI = () => <ExtendedCompWithExtraProps2 behavior="test" />
+/* should not error */
+export const ExtendJ = () => <ExtendedCompWithExtraProps2 behavior="padding" />
+/* @ts-expect-error custom cannot be string */
+export const ExtendK = () => <ExtendedCompWithExtraProps2 custom="test" />
+/* should not error */
+export const ExtendL = () => <ExtendedCompWithExtraProps2 custom={false} />


i feel like all this type testing should belong somewhere else other than examples/example/App.tsx... some kind of tests.tsx file somewhere. let me know if you'd like me to move it all.

yeah this should get cleaned up at some point. examples isn't a great name for this, it's really just for local dev testing. i point people to the solito starter instead for using dripsy

nandorojo · 2022-09-15T13:40:44Z

this behavior looks right! i'll take a look today

jjenzz · 2022-09-15T13:45:09Z

@nandorojo great, thanks ❤️

also, just a heads up that for some reason installing older versions of dripsy installs the latest versions of types so my project has errors everywhere that i cannot silence with a downgrade 🙈

not sure why older versions don't pull types specific to that version but something for you to look into separately perhaps?

nandorojo · 2022-09-15T13:55:58Z

hmm it could be something related to the monorepo setup perhaps...not sure. hopefully publishing the newer version will fix it all

nandorojo · 2022-09-15T13:56:30Z

if you also have gradient installed, be sure to fix it at the same version. i'm going to fix this in v4

jjenzz · 2022-09-15T14:07:26Z

@nandorojo i think it may have been something to do with local caching (pnpm maybe 🤷 ). i've completely removed a bunch of stuff and reinstalled 3.6.0 and have the correct types, so that's a relief!

nandorojo · 2022-09-15T14:23:57Z

just published an updated version (3.8.1) with these changes

nandorojo · 2022-12-26T16:37:47Z

Hey @jjenzz! I think one of these PRs caused a type regression for ScrollView and TextInput. I can't quite figure out why. My best guess is that they have some unique ref type, since both of these components have custom methods like scrollTo() and focus(). That cause might be a red herring, but it would make sense to me...

Here's the issue: #260

Thank you (& happy holidays!)

fix exported types

62f3020

vercel bot had a problem deploying to Preview September 14, 2022 21:27 Failure

jjenzz marked this pull request as draft September 14, 2022 21:39

jjenzz marked this pull request as ready for review September 14, 2022 21:41

vercel bot had a problem deploying to Preview September 15, 2022 07:56 Failure

vercel bot had a problem deploying to Preview September 15, 2022 07:59 Failure

jjenzz marked this pull request as draft September 15, 2022 08:00

vercel bot had a problem deploying to Preview September 15, 2022 08:08 Failure

reinstate props generic

1969f02

jjenzz marked this pull request as ready for review September 15, 2022 08:16

vercel bot had a problem deploying to Preview September 15, 2022 08:16 Failure

jjenzz commented Sep 15, 2022

View reviewed changes

adjust error comments

2b67ed9

vercel bot had a problem deploying to Preview September 15, 2022 14:22 Failure

nandorojo merged commit c979df6 into nandorojo:master Sep 15, 2022

jjenzz deleted the fix-exported-types branch September 15, 2022 14:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix exported types #239

fix exported types #239

jjenzz commented Sep 14, 2022 •

edited

Loading

vercel bot commented Sep 14, 2022 •

edited

Loading

nandorojo commented Sep 14, 2022

jjenzz commented Sep 14, 2022 •

edited

Loading

jjenzz commented Sep 14, 2022 •

edited

Loading

jjenzz commented Sep 15, 2022 •

edited

Loading

jjenzz Sep 15, 2022

nandorojo Sep 15, 2022

nandorojo commented Sep 15, 2022

jjenzz commented Sep 15, 2022 •

edited

Loading

nandorojo commented Sep 15, 2022

nandorojo commented Sep 15, 2022

jjenzz commented Sep 15, 2022 •

edited

Loading

nandorojo commented Sep 15, 2022

nandorojo commented Dec 26, 2022 •

edited

Loading

fix exported types #239

fix exported types #239

Conversation

jjenzz commented Sep 14, 2022 • edited Loading

DIFF

vercel bot commented Sep 14, 2022 • edited Loading

nandorojo commented Sep 14, 2022

jjenzz commented Sep 14, 2022 • edited Loading

jjenzz commented Sep 14, 2022 • edited Loading

jjenzz commented Sep 15, 2022 • edited Loading

jjenzz Sep 15, 2022

Choose a reason for hiding this comment

nandorojo Sep 15, 2022

Choose a reason for hiding this comment

nandorojo commented Sep 15, 2022

jjenzz commented Sep 15, 2022 • edited Loading

nandorojo commented Sep 15, 2022

nandorojo commented Sep 15, 2022

jjenzz commented Sep 15, 2022 • edited Loading

nandorojo commented Sep 15, 2022

nandorojo commented Dec 26, 2022 • edited Loading

jjenzz commented Sep 14, 2022 •

edited

Loading

vercel bot commented Sep 14, 2022 •

edited

Loading

jjenzz commented Sep 14, 2022 •

edited

Loading

jjenzz commented Sep 14, 2022 •

edited

Loading

jjenzz commented Sep 15, 2022 •

edited

Loading

jjenzz commented Sep 15, 2022 •

edited

Loading

jjenzz commented Sep 15, 2022 •

edited

Loading

nandorojo commented Dec 26, 2022 •

edited

Loading