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

react admin error with Menu.Item missing properties #9737

Closed
charleschouette opened this issue Mar 20, 2024 · 17 comments · Fixed by #9853
Closed

react admin error with Menu.Item missing properties #9737

charleschouette opened this issue Mar 20, 2024 · 17 comments · Fixed by #9853
Labels

Comments

@charleschouette
Copy link

I got an error when using the component Menu.Item of react admin. It seems that all the properties are missing.
Therefore I can't build my app without adding all the 250+ properties of Menu.Item. I noticed that it works when using the component MenuItemLink.
You can reproduce it by using the documentation example of Menu.Item. I am using node version v21.6.1 and npm version 10.2.4

it does not build with the code:

// in src/MyMenu.js
import { Menu } from 'react-admin';

export const MyMenu = () => (
    <Menu>
        ...
        <Menu.Item to="/custom-route" primaryText="Miscellaneous" />
    </Menu>
);
  • React-admin version: 4.16.13
  • React version:18.2.0
  • Stack trace:
error TS2739: Type '{ to: string; primaryText: string; }' is missing the following properties from type 'Pick<MenuItemLinkProps, "children" | "replace" | "value" | "id" | "className" | "dense" | "title" | "defaultChecked" | "defaultValue" | "suppressContentEditableWarning" | ... 277 more ... | "tooltipProps">': onPointerEnterCapture, onPointerLeaveCapture

it seems related to this issue: #1196 that has already been resolved but adding @types/react-router and @types/react-router-dom did not solve the problem.

@fzaninotto
Copy link
Member

Thanks for your report.

I can somehow reproduce it in the simple example (https://stackblitz.com/edit/github-1ymznm?file=src%2FLayout.tsx), but not in the monorepo. There must be a regression somewhere in one of the dependencies.

@fzaninotto fzaninotto added the bug label Mar 20, 2024
@fzaninotto
Copy link
Member

The two missing props seem to be onPointerEnterCapture and onPointerLeaveCapture

@fzaninotto
Copy link
Member

fzaninotto commented Mar 20, 2024

The problem comes from @types/react.

  • It disappears if you downgrade to 18.2.65.
  • It reappears if you use 18.2.66.
  • It's still present in the latest version (18.2.67).

@fzaninotto
Copy link
Member

This is indeed a problem with DefinitelyTyped: DefinitelyTyped/DefinitelyTyped#69006

The fix has been merged and should be released soon: DefinitelyTyped/DefinitelyTyped#69005

In the meantime, please limit the @types/react version in your packages.json.

@Pharb
Copy link

Pharb commented Mar 25, 2024

The fix has been merged and should be released soon: DefinitelyTyped/DefinitelyTyped#69005

I think this is not enough, as it only reverts the type removal for v16 and v17 but does not changes anything about v18:
DefinitelyTyped/DefinitelyTyped@1bfbd3b

The error still appears with the latest v18.2.70 released today.

Is this solved with the next bugfix release of react-admin v4? Or will it only work with react-admin v5, which includes the react v18 update?

@fzaninotto
Copy link
Member

react-admin can't fix this regression, as it comes from DefinitelyTyped. I suggest you downgrade to 18.2.65, and report the fact that the bug is still present in the DefinitelyTyped repo.

@fzaninotto
Copy link
Member

The problem seems to be still present in @types/react 18.2.78 and 18.3.1.

@AntonOfTheWoods
Copy link
Contributor

@fzaninotto , unfortunately it looks like there was a "fix" that was subsequently abandoned. Given that, and the attitude of the devs working on the @types/react, it seems very unlikely this will be "fixed" -> they said it caused too many issues so they reverted. I think the safe bet would be to remove it.

@fzaninotto
Copy link
Member

@AntonOfTheWoods Can you elaborate on the attitude of the devs? Also, what would you want to "remove"?

@fzaninotto
Copy link
Member

So accoding to DefinitelyTyped/DefinitelyTyped#69243 (review), the React types won't be updated for React 18, so we need to fix it in react-admin.

@fzaninotto
Copy link
Member

fzaninotto commented May 13, 2024

I can't reproduce the problem anymore with React 18 (in react-admin branch next). It seems to be fixed upstream.

If you still have the bug, can you give a repro? Note that React 18 isn't officially supported by react-admin v4.

@ilia-os
Copy link
Contributor

ilia-os commented May 13, 2024

@fzaninotto Fix in react-admin next is great news, but what about current fresh installs? They all are partially broken type-wise, right?

create-react-admin sets up a project with react 18 - everyone assumes it is supported (more - it is recommended). Also there is no big red note about it being not officially supported anywhere in the docs, that I noticed)

It's not only MenuLinkItem affected, same behavior with InspectorButton for example

@fzaninotto
Copy link
Member

Apologies, my mind is confused by switching back and forth between versions.

You're right, react-admin v4, the current version, does support React 18. So we need to fix this issue in the master branch.

The fact that the issue is (apparently) already fixed in the next branch isn't enough.

@ilia-os
Copy link
Contributor

ilia-os commented May 13, 2024

I am willing to help, yet i'm not sure what the fix would be. At least i'll confirm it will resolve)
I've found this issue from ant-design though ant-design/ant-design-icons#631 - they removed all forwardRef calls

Also, InspectorButton requires one additinal prop "placeholder", which seems useless. Its being passed to mui IconButton which uses forwardRef
But ResourceMenuItem, which relies on MenuItemLink (which also uses forwardRef) does not have this issue and works fine...
Maybe we could provide "safe" variants without top-level forwardRef + update unsafe components jsdoc, or just overwrite the component types..

image

@fzaninotto
Copy link
Member

It would be a great gelp if you could list here the components that trigger the compilation error.

@ilia-os
Copy link
Contributor

ilia-os commented May 16, 2024

I've looked up by forwardRef usage, and most of the components work fine.
Another affected component I found is ResettableTextField - it also requires props onPointerEnterCapture, onPointerLeaveCapture. So currently there are 3 components:

  1. MenuItemLink - requires onPointerEnterCapture, onPointerLeaveCapture
  2. ResettableTextField - requires onPointerEnterCapture, onPointerLeaveCapture
  3. InspectorButton - requires onPointerEnterCapture, onPointerLeaveCapture and placeholder

As I see, placeholder field from MenuItemLink is already omitted...

@AntonOfTheWoods
Copy link
Contributor

Also, what would you want to "remove"?

#9853 works @fzaninotto

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants