-
Notifications
You must be signed in to change notification settings - Fork 175
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
Declare relative-element
under React.JSX to fix React 19 compatibility
#305
base: main
Are you sure you want to change the base?
Conversation
Fixes github#304 I had to add a `@ts-expect-error` comment since otherwise the typechecker would complain about missing `react` as a dependency.
@github/primer you'll need to integrate this to check whether or not this is viable for the monolith. Please do not merge before checking this, otherwise we'll have to revert. |
@eramdam looks like this isn't quite passing the CI checks. Does |
Oops, you're right, it doesn't 🙈 I wanted to avoid it but it looks like the only clean way of fixing the issue was to add |
Co-authored-by: Keith Cirkel <[email protected]>
Hey @eramdam, thanks for looking into this. I'm hesitant to merge React-specific code into this project, considering it really has nothing to do with React. Could we instead put the React code into a separate .d.ts file and ask that clients add it to their tsconfig.json? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this looks okay from a library perspective. I dislike that there’s react specific idioms too, but react is always messy to integrate with so… 🤷♂️
@camertron lets try integrating this with PVC and see if there are any issues. AIUI this shouldn’t cause too much friction within PVC, but it might and at that point we’ll need to look at other solutions. My hope is this is a transparent change to non-react projects like PVC, and doesn’t pull in additional dependencies.
Fixes #304
I had to add a
@ts-expect-error
comment since otherwise the typechecker would complain about missingreact
as a dependency.