mdx icon indicating copy to clipboard operation
mdx copied to clipboard

Type Error on jsx type with react jsx runtime

Open mghahari opened this issue 1 year ago • 7 comments

Initial checklist

Affected packages and versions

3.0.1

Link to runnable example

https://stackblitz.com/edit/vitejs-vite-ahedk8?file=src%2Fmain.tsx

Steps to reproduce

You can see type error in the sandbox for options parameter of the evaluate function. (in main.tsx file)

Expected behavior

Doesn't have type error with evaluate function when we run the sample code from documentation:

import {evaluate} from '@mdx-js/mdx'
import * as runtime from 'react/jsx-runtime'

console.log(await evaluate(file, runtime))

Actual behavior

Type error:

Argument of type 'typeof import("stackblitz:/node_modules/@types/react/ts5.0/jsx-runtime")' is not assignable to parameter of type 'Readonly<EvaluateOptions>'.
  Types of property 'jsx' are incompatible.
    Type '(type: ElementType<any, keyof IntrinsicElements>, props: unknown, key?: Key | undefined) => ReactElement<any, string | JSXElementConstructor<any>>' is not assignable to type 'Jsx'.
      Types of parameters 'type' and 'type' are incompatible.
        Type 'unknown' is not assignable to type 'ElementType<any, keyof IntrinsicElements>'.

Runtime

No response

Package manager

No response

OS

No response

Build and bundle tools

No response

mghahari avatar Apr 05 '24 03:04 mghahari

Heya! Yeah, react did not have types. So there was an error already because types were missing. Now it has weird new types, and there’s still an error. You can add a // @ts-expect-error like before. Or, PRs wanted, to improve the types of jsx and such!

wooorm avatar Apr 05 '24 09:04 wooorm

Thanks for your response @wooorm. According to your response, the main problem is in the type definitions of the react library. I reviewed the type definitions for jsx in the DefinitelyTyped repository: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f1da3cee5648005e07550ae729d4aadee764da3b/types/react/jsx-runtime.d.ts#L22

It seems that it has reasonable types. But it's specific to react. For mdx-js/mdx, after a non-straightforward journey, I found this for type definition: https://github.com/syntax-tree/hast-util-to-jsx-runtime/blob/05c087bef85f6dd6a1f1a09abc0aef89236f7f69/lib/index.js#L93

It doesn't have a specific definition for the type property. It's maybe reasonable because it's framework agnostic.

I think any PR for react types will not be accepted because it has reasonable type in its' context. And we cannot make a PR for hast-util-to-jsx-runtime because it should be framework agnostic.

Do you have any idea for solving this problem?

mghahari avatar Apr 05 '24 10:04 mghahari

And we cannot make a PR for hast-util-to-jsx-runtime because it should be framework agnostic.

There might be something doable there? Or, Remco’s PR mentioned above. Though I wonder if there can be some types between no types vs broken types. https://github.com/mdx-js/mdx/pull/2465#discussion_r1553974317

wooorm avatar Apr 05 '24 16:04 wooorm

@remcohaszing, @wooorm Thank you for investigating on this issue. I don't have enough information about this library for suggesting a feasible solution. In general, I think we should define a minimal interface that any compatible type can satisfies it. In this case we don't force a complete and opinionated type and just what's enough to this library works correctly. But I don't know it's possible to do that in jsDoc format.

mghahari avatar Apr 05 '24 18:04 mghahari

Yup! (and: almost everything is possible with jsdoc as in plain ts! and if it isn‘t, we can use plain d.ts files!)

wooorm avatar Apr 05 '24 19:04 wooorm

Run into the same problem.

artuska avatar Jun 07 '24 09:06 artuska

makes sense; answer is the same too

wooorm avatar Jun 07 '24 09:06 wooorm