material-ui
material-ui copied to clipboard
Bump @types/react to v18.3.5
This PR contains the following updates:
| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| @types/react (source) | ^18.3.4 -> ^18.3.5 |
||||
| @types/react (source) | 18.3.4 -> 18.3.5 |
Configuration
📅 Schedule: Branch creation - "on sunday before 6:00am" in timezone UTC, Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about these updates again.
- [ ] If you want to rebase/retry this PR, check this box
This PR was generated by Mend Renovate. View the repository job log.
Netlify deploy preview
https://deploy-preview-43555--material-ui.netlify.app/
Bundle size report
Details of bundle changes (Toolpad) Details of bundle changes
Generated by :no_entry_sign: dangerJS against 97bacdd6f6ce7002f0b16f6ac45108b7ff3357de
@michaldudak this @types/react version breaks how we handled polymorphic components in @mui/base (see https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70361). We could freeze the @mui/base version of @types/react, but we need to solve BasePopper.tsx, which was copied to @mui/material to remove the @mui/base dependency. For example, this removes the error:
-const PopperTooltip = React.forwardRef(function PopperTooltip<
+const PopperTooltip = React.forwardRef<HTMLDivElement, PopperTooltipProps>(function PopperTooltip<
RootComponentType extends React.ElementType,
>(props: PopperTooltipProps<RootComponentType>, forwardedRef: React.ForwardedRef<HTMLDivElement>) {
(source)
But it seems to me like this breaks the polymorphic usage of the RootComponentType generic. I'm not that familiar with the PolymorphicProps type so I'm not sure.
Is this the correct fix?
Since we're casting the result of forwardRef to PolymorphicComponent<PopperTypeMap>, the change you're proposing should be safe and not noticeable for external users.
@michaldudak I tried the fix here, but more issues showed up, I'll have to dig deeper.
I don't know how to apply the fix to this one, which uses a generic: https://github.com/mui/material-ui/pull/43555/files#diff-d20c7994b87da6016e5837861c4dbf679747074bf2706031cc9157de13ab056dR24
cc: @Janpot @aarongarciah in case you have some time this week to look into these issues.
Edited/Blocked Notification
Renovate will not automatically rebase this PR, because it does not recognize the last commit author and assumes somebody else may have edited the PR.
You can manually request rebase by checking the rebase/retry box above.
⚠️ Warning: custom changes will be lost.
@DiegoAndai This seems to be the naive solution. Wouldn't that work?
Thanks @Janpot! looks good to me.
Does this mean we can revert my commit? https://github.com/mui/material-ui/pull/43555/commits/a5548f54cd9102aa350ff16e48ee905709de374b
Does this mean we can revert my commit?
I believe so, we can solve it the same way by making the ref optional.
I believe so, we can solve it the same way by making the ref optional.
Thanks! I'll do it and update the PR
@DiegoAndai I just rebased on top of master and removed your commit. I need this PR to fix some stuff https://github.com/mui/material-ui/pull/43964#issuecomment-2388004167 I'll try to apply the necessary changes. Let's see if we can land this PR today.
@aarongarciah @Janpot I had to bring back my fix (this one): https://github.com/mui/material-ui/pull/43555/commits/dd9c8febf50b2692ee80d499194981d75b1a38a3 and https://github.com/mui/material-ui/pull/43555/commits/bb9f21bbef23ec65b8c90d22fa823dd4bd673695
Seems to me that this particular problem can't be fixed with the approach in https://github.com/mui/material-ui/pull/43792, but instead is caused by something not working with forward ref inference and our polymorphic types after https://github.com/DefinitelyTyped/DefinitelyTyped/pull/70361. Please correct me if I'm wrong.
Because this issue is mainly inside mui-base, which is stale, and one material component that should be eventually removed, plus the components being casted anyways, I would merge this one to unblock the release.
@DiegoAndai it's fine for me. I see we need to bring the changes in the docs Next.js config file because after pnpm dedupe, next points to the newest 14.x.x version (see error https://app.netlify.com/sites/material-ui/deploys/66fd4632cd5fe8000889b866#L295)