material-ui icon indicating copy to clipboard operation
material-ui copied to clipboard

Bump @types/react to v18.3.5

Open renovate[bot] opened this issue 1 year ago • 3 comments

This PR contains the following updates:

Package Change Age Adoption Passing Confidence
@types/react (source) ^18.3.4 -> ^18.3.5 age adoption passing confidence
@types/react (source) 18.3.4 -> 18.3.5 age adoption passing confidence

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.

renovate[bot] avatar Sep 01 '24 01:09 renovate[bot]

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

mui-bot avatar Sep 01 '24 01:09 mui-bot

@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?

DiegoAndai avatar Sep 04 '24 20:09 DiegoAndai

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 avatar Sep 06 '24 11:09 michaldudak

@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.

DiegoAndai avatar Sep 16 '24 20:09 DiegoAndai

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.

renovate[bot] avatar Sep 16 '24 23:09 renovate[bot]

@DiegoAndai This seems to be the naive solution. Wouldn't that work?

Janpot avatar Sep 17 '24 16:09 Janpot

Thanks @Janpot! looks good to me.

Does this mean we can revert my commit? https://github.com/mui/material-ui/pull/43555/commits/a5548f54cd9102aa350ff16e48ee905709de374b

DiegoAndai avatar Oct 01 '24 15:10 DiegoAndai

Does this mean we can revert my commit?

I believe so, we can solve it the same way by making the ref optional.

Janpot avatar Oct 01 '24 15:10 Janpot

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 avatar Oct 01 '24 15:10 DiegoAndai

@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 avatar Oct 02 '24 10:10 aarongarciah

@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 avatar Oct 02 '24 13:10 DiegoAndai

@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)

aarongarciah avatar Oct 02 '24 13:10 aarongarciah