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

Potential TS bug when using a Box as other components #120

Open
2 tasks done
richbachman opened this issue Feb 26, 2023 · 8 comments
Open
2 tasks done

Potential TS bug when using a Box as other components #120

richbachman opened this issue Feb 26, 2023 · 8 comments
Labels
bug Something isn't working

Comments

@richbachman
Copy link
Contributor

Description

I'm running into a TypeScript error when using a Box component built with rainbow-sprinkles as a Button. Here is a screenshot of the error:

Screenshot 2023-02-26 at 3 24 40 PM

Here is the Box component: https://github.com/richbachman/vega-ui/blob/main/packages/ui/src/primitives/Box/src/index.tsx
Here are the atoms I'm passing to Rainbox Sprinkles: https://github.com/richbachman/vega-ui/blob/main/packages/ui/src/styles/style-props.css.ts

The Button component consists of a main component which loads separate components depending on the variant:

I know I'm passing a lot of dynamicProperties in the defineProperties function. Maybe too many?

Let me know if you have any ideas on why this might be happening. Thanks!

Also note this is very much a WIP, so there's some commented code and other libraries being used to test different ideas. I'd really like to stick with rainbow-sprinkles though.

Expected Behaviour

I expect there to be no TypeScript error.

Actual Behaviour

There's a TypeScript error. This only appears to happen when type checking and in VSCode. The Storybook and the build will run fine.

Affected Version

0.15.2

Steps to Reproduce

  1. Feel free to pull the repo
  2. Run pnpm install
  3. Run pnpm build
  4. Run pnpm type-check to see the error

Checklist

@roginfarrer
Copy link
Collaborator

Thanks for laying out all of the details. I followed your steps to reproduce, and I don't get any errors when running pnpm type-check. I also can't find that hover error any where in the project. Maybe try deleting node_modules and rerunning install? Or restarting your editor?

@richbachman
Copy link
Contributor Author

@roginfarrer thanks for taking a look. I just deleted node_modules and reinstalled everything. I also restarted VSCode, but am still seeing the error. Very perplexing on this one. I have to knock out some other things for work today but will pull down a fresh version of the repo and see if I get it there.

@roginfarrer
Copy link
Collaborator

Do you only get this error in the editor? Or also when running pnpm type-check?

@richbachman
Copy link
Contributor Author

It looks like its only in VSCode. So must be something strange going on there.

@roginfarrer
Copy link
Collaborator

Maybe make sure that VSCode is up to date, and that VSCode's TS server is using the same version as the project's.

@richbachman
Copy link
Contributor Author

@roginfarrer if you don't mind, try one last thing. Make a change in the PrimaryButton-box.tsx file. Something like adding textAlign="center" and see if that makes VSCode show the error. Thanks!

@awdyson
Copy link

awdyson commented Apr 4, 2023

For VSCode, make sure you're using your workspace version of Typescript:
image


I also ran into this issue when trying to form an intersection of very large types -- the kind that this library produces.
Setting said intersection to its own type before adding it to a component's props cleared the error for me.
image
Looking at your code, maybe playing around with how BoxProps is assembled will help?
I am very far from a TS expert, but I could see the sheer number of props in StyleProps and ComponentPropsWithoutRef causing issue.

Personally I have a generic to handle the polymorphism (you can ignore the FC-handling):
image
image

@roginfarrer roginfarrer added the bug Something isn't working label Apr 13, 2023
@omarkhatibco
Copy link

omarkhatibco commented Apr 26, 2023

Hey everyone,

I spent the last 2 days trying to solve this problem , thanks for @awdyson I got a working version from his screenshots

therefore I would like to share my version of the Box which supports the ref props as well

types based on @awdyson screenshots

import { ComponentPropsWithoutRef, ElementType, PropsWithChildren } from 'react'

export type As = ElementType
export type Props = Record<string, unknown>

export type Polymorphic<E extends As, P = Props> = P &
	PropsWithChildren<{
		as?: E
	}> &
	ComponentPropsWithoutRef<E>

export type InferredElementProps<C extends ElementType> = C extends keyof JSX.IntrinsicElements
	? JSX.IntrinsicElements[C]
	: never

the box component

import { ForwardedRef, ReactElement, createElement, forwardRef } from 'react'
import { Sprinkles, rainbowSprinkles } from '@/system'
import { ClassValue, clsx } from 'clsx'

import { As, InferredElementProps, Polymorphic } from '@/types'

export type BoxProps = Sprinkles & {
	testId?: string
	className?: ClassValue
}

// Box Component
export const BoxComponent = <E extends As>(
	{ as, className: externalClassNames, children, testId, ...props }: Polymorphic<E, BoxProps>,
	ref: ForwardedRef<InferredElementProps<E>>,
) => {
	const tag = as || 'div'
	const { className: internalClassNames, style, otherProps } = rainbowSprinkles(props)

	const className = clsx(externalClassNames, internalClassNames)

	return createElement(
		tag,
		{ ref, className, style, 'data-testid': testId, ...otherProps },
		children,
	)
}

// Box Wrapper
export const Box = forwardRef(BoxComponent) as <E extends As>(
	p: Polymorphic<E, BoxProps> & {
		ref?: ForwardedRef<InferredElementProps<E>>
	},
) => ReactElement | null

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants