palantir / blueprint

A React-based UI toolkit for the web
https://blueprintjs.com/
Apache License 2.0
20.65k stars 2.17k forks source link

`React.JSX.Element` type in select `ItemRenderer` type is not compatible with`@types/react` < 18.2.6 #6841

Open MatthewHerbst opened 4 months ago

MatthewHerbst commented 4 months ago

Environment

Code Sandbox

Link to a minimal repro (fork this code sandbox):

Steps to reproduce

  1. Install the packages listed
  2. Run your types build

Actual behavior

Trying to build types failed:

node_modules/@blueprintjs/select/src/common/itemRenderer.ts:72:80 - error TS2694: Namespace 'React' has no exported member 'JSX'.

72 export type ItemRenderer<T> = (item: T, itemProps: ItemRendererProps) => React.JSX.Element | null;
                                                                                  ~~~

Found 1 error in node_modules/@blueprintjs/select/src/common/itemRenderer.ts:72

Expected behavior

There are no type errors

Possible solution

React.JSX.Element was introduced in@types/react@18.2.6. This makes its use incompatible with the @blueprintjs/select@5.1.5 package's peerDependencies which specifies:

  "peerDependencies": {
    "@types/react": "^16.14.41 || 17 || 18",
    "react": "^16.8 || 17 || 18",
    "react-dom": "^16.8 || 17 || 18"
  },

Older versions of React could be supported by changing from React.JSX.Element -> JSX.Element, though I have only tested with @types/react@17.0.53. I have not tested with @types/react@16.14.41.

gluxon commented 3 months ago

Thanks for reporting a bug!

React.JSX.Element was introduced in@types/react@18.2.6.

It looks like that change also edited @types/react versions v15, v16, and v17. 🤔

Adi updated the minimum Blueprint supported version to v16.14.41 here: https://github.com/palantir/blueprint/pull/6634

@types/react is now on the latest v16 typings. There was a change to deprecate the global JSX namespace in favor of the React.JSX namespace in May 2023 via https://github.com/DefinitelyTyped/DefinitelyTyped/commit/f1b25591890978a92c610ce575ea2ba2bbde6a89, which was released in v16.14.41, so that is now the new minimum supported version of React types.

I think the right fix here is to be more specific Blueprint's @types/react peer dependencies and include minimum versions for 17.x and 18.x as well. Example:

{
  "peerDependencies": {
    "@types/react": "^16.14.41 || ^17.0.59 || ^18.2.6"
  }
}

I'm hesitant to go back to the unnamespaced JSX.Element since that's deprecated, even on older versions of @types/react.

@MatthewHerbst Does the above make sense? Could you upgrade your project to @types/react@17.0.59 or greater?

MatthewHerbst commented 3 months ago

Thanks for the response, @types/react@17.0.59 works!

My main reasoning for suggesting returning to JSX.Element was to avoid any potential semver major version change issue (not sure if that's required for the peer dep change being asked for, but I think technically it is?). Will let y'all decide on that though, a patch version with clear docs about why folks might be getting new peer dep warnings is probably fine since they don't need to make a major version change to @types/react. Thanks!