livekit / components-js

Official open source React components and examples for building with LiveKit.
https://livekit.io
Apache License 2.0
149 stars 64 forks source link

@livekit/components-react rewrites the type of forwardRef #837

Closed robintown closed 4 months ago

robintown commented 4 months ago

Select which package(s) are affected

\@livekit/components-react

Describe the bug

This redeclaration of forwardRef causes application code to break when upgrading to @livekit/components-react v2.1.0.

Example breakage: https://github.com/element-hq/element-call/actions/runs/8752293413/job/24019624702?pr=2323

Reproduction

If your application has @livekit/components-react anywhere in its dependency graph, and it includes application code that looks like this, you'll get a type error:

import { forwardRef } from 'react'
const ExampleComponent = forwardRef<HTMLElement>(/* ... */)
ExampleComponent.displayName = 'ExampleComponent'

This is because @livekit/components-react is rewriting the type of forwardRef, and not allowing its return value to have a displayName property. As a consumer of @livekit/components-react, I have no good way to opt out of its redeclaration of React's types.

I have no context for why this was done, but if it's necessary to use some other type signature for forwardRef internally, I suggest casting it without the use of any declare blocks, so as to avoid polluting a type namespace which doesn't belong to LiveKit:

import { forwardRef } from 'react'
export const ourDifferentlyTypedForwardRef = forwardRef as unknown as OurDifferentType

Logs

No response

System Info

Not relevant

Severity

blocking an upgrade

Additional Information

No response

lukasIO commented 4 months ago

Sorry about that. The redeclaration was an attempt to fix type errors occuring from the fact that we need to re-use forwarded refs within components. However that approach was flawed from the start and we have since replaced it with the recommended useImperativeHandle. We should have removed the redeclaration as part of that change, but it got lost.

robintown commented 4 months ago

Cool, thanks for the quick fix!