rage / mooc.fi

https://mooc.fi
5 stars 6 forks source link

fix MUI component augmentation for next/link #1123

Closed mipyykko closed 1 year ago

mipyykko commented 1 year ago

We've been using the next/link behavior in the MUI elements such as Link, Button, MenuItem and indeed everything that extends ButtonBase. To make Typescript happy with merging of the two prop types, I had augmented the existing types -- but apparently this hadn't worked as I thought it would because there were default exports and augmentation didn't work with those and promptly made the prop type for any of those elements any. That's why the front page course card links broke when I removed Google Analytics -- TS didn't tell me I had forgotten to change one to to href in the card element props.

This change is not very optimal as it depends on some manual typing, but if some Link, Button etc. is being difficult with non-compatible props that come from next/link (such as shallow, prefetch etc.), you can type the element with EnhancedLink, EnhancedButton and so on. The generic types follow MUI element types - EnhancedButton<"div", MyOwnButtonProps> would tell TS to expect props usually passed to a div (instead of a button), as well as the custom props specified.

codecov[bot] commented 1 year ago

Codecov Report

Merging #1123 (b0f8cdd) into master (0b36fa0) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1123   +/-   ##
=======================================
  Coverage   67.80%   67.80%           
=======================================
  Files         117      117           
  Lines        4696     4696           
  Branches     1025     1025           
=======================================
  Hits         3184     3184           
  Misses       1402     1402           
  Partials      110      110           
Flag Coverage Δ
backend 67.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more