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

Type error reported by @typescript-eslint/no-unsafe-return #10

Closed
4 tasks done
karlhorky opened this issue Dec 6, 2024 · 8 comments
Closed
4 tasks done

Type error reported by @typescript-eslint/no-unsafe-return #10

karlhorky opened this issue Dec 6, 2024 · 8 comments
Labels
👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on

Comments

@karlhorky
Copy link

Initial checklist

Affected package

hast-util-to-jsx-runtime@2.3.2

Steps to reproduce

  1. Configure @typescript-eslint/no-unsafe-return rule in some Node.js or Next.js project (eg. Next.js App Router project)
  2. Add Shiki example code for hast-util-to-jsx-runtime in components/CodeBlock.tsx
  3. 💥 Observe "Unsafe return of a value of type error" error with return value

Reproduction (typescript-eslint Playground)

Unsafe return of a value of type error. 12:3 - 20:6

Screenshot 2024-12-06 at 15 11 13

Using a as JSX.element type assertion on the return value of toJsxRuntime() is a workaround for the issue:

Screenshot 2024-12-06 at 15 11 26

Actual behavior

Error with a "type error". The type error is not observable.

Expected behavior

No error

Runtime

node@22.11.0

Package manager

yarn@1.22.22

Operating system

macOS Sequoia 15.1.1

Build and bundle tools

other (please specify in steps to reproduce)

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Dec 6, 2024
@karlhorky
Copy link
Author

karlhorky commented Dec 6, 2024

Maybe related to #6 ? (now that React 19 has been released)

@remcohaszing
Copy link
Member

This is an XY-problem. Your linter complains. The real problem is that there is a type error in hast-util-to-jsx-runtime when using it with modern JSX frameworks, which now include React 19. You’re just suppressing the type error.

This is indeed what I tried to fix in #6. As a workaround you can define the global JSX namespace yourself, do a type cast, or add an eslint-disable comment. I recommend type-casting to ReactElement.

@karlhorky
Copy link
Author

karlhorky commented Dec 6, 2024

I recommend type-casting to ReactElement.

Oh ok, that would diverge from the types of hast-util-to-jsx-runtime, which report a return type of JSX.Element:

Screenshot 2024-12-06 at 15 37 29

JSX.Element seems to be the more common return type of React components:

Screenshot 2024-12-06 at 15 38 24

I think I'll stick with the type assertion workaround I mentioned in the original issue above:

import { type JSX } from 'react';

toJsxRuntime(...) as JSX.Element

@karlhorky
Copy link
Author

To be clear, that's just a workaround though - does hast-util-to-jsx-runtime intend to fix this issue too?

I can't observe the actual TS type error in my editor / on the command line via tsc weirdly enough, only typescript-eslint can see it.

@remcohaszing
Copy link
Member

The JSX namespace is a special internal type to tell TypeScript how JSX works. They are obvervable to React users, but they’re not really intended for public use. In React, JSX.Element is an alias of `ReactElement.

JSX frameworks used to define the JSX namespace globally. This was really convenient for types that work with JSX in a generic manner, such as hast-util-to-jsx and @types/mdx. But over recent years all major JSX frameworks have moved away from the global JSX namespace, React being the last one with React 19.

IMO we should not rely on the global JSX namespace anymore at all, but the team is in disagreement on this. Also even if we do, the problem is hard to resolve. If we can’t rely on the JSX namespace, a lot of types internally become any/unknown.

@wooorm
Copy link
Member

wooorm commented Dec 6, 2024

This project depends on JSX, which is why it uses the quite useful JSX types. Type them and it works. Or don’t, then there are no types. I would really appreciate it if people talk to TS/React folks who want to go without these useful types. There are several other places where this can be discussed, and this ESLint rule isn’t it.

@wooorm wooorm closed this as completed Dec 6, 2024
@wooorm wooorm added the 👀 no/external This makes more sense somewhere else label Dec 6, 2024

This comment has been minimized.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Dec 6, 2024
@karlhorky
Copy link
Author

karlhorky commented Dec 6, 2024

Thanks for the background!

This project depends on JSX, which is why it uses the quite useful JSX types. Type them and it works.

@wooorm I'm guessing that you mean ae638ba, the recommendation in the Use section of the readme to use the following code:

import type {JSX as Jsx} from 'react/jsx-runtime'

declare global {
  namespace JSX {
    type ElementClass = Jsx.ElementClass
    type Element = Jsx.Element
    type IntrinsicElements = Jsx.IntrinsicElements
  }
}

Screenshot 2024-12-06 at 19 01 13

It seems like it's kind of ok, but it's a lot of code for common use cases, and the error DX is not great (finding the error first of all, and then finding out that it's not a bug but a separate setup requirement - to paste code in some file).

Since there is the widespread removal of the global JSX interface, it does feel like the generic option would be slightly better - I'm guessing it would lead to code like this?

import { type JSX } from 'react';

toJsxRuntime<JSX>(...)

Or maybe there are other options:

I'm not sure whether this was considered somewhere (maybe already in #6, if I didn't understand completely yet), but would it be possible to retrieve the type from the passed jsx function somehow instead?

So that no code would need to change, and the type is somehow retrieved from the function?

import { toJsxRuntime } from 'hast-util-to-jsx-runtime';
import { Fragment } from 'react';
import { jsx, jsxs } from 'react/jsx-runtime';

toJsxRuntime(out, {
  Fragment,
  jsx, // Type retrieved from this `jsx` function
  jsxs,
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

3 participants