Change the typings of Clock to use a static type as ClassKey

[mvestergaard]

This PR closes #1465

Description

This fixes some compatibility issues with material-ui 4.9.0
It should also be backwards compatible

The changes to docs/prop-types.json was done automatically by some sort of tooling when i committed... Not sure if the tooling isn't compatible with windows or something.
I managed to circumvent the tooling that generates and adds prop-types.json to the commit.

[codecov]

Codecov Report

Merging #1466 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #1466      +/-   ##
=========================================
+ Coverage   93.62%   93.7%   +0.07%     
=========================================
  Files          55      55              
  Lines        1350    1350              
  Branches      199     199              
=========================================
+ Hits         1264    1265       +1     
+ Misses         68      67       -1     
  Partials       18      18

Impacted Files	Coverage Δ
lib/src/views/Clock/Clock.tsx	96% <ø> (ø)	⬆️
lib/src/views/Clock/ClockPointer.tsx	96% <0%> (+4%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a865eaa...523e3ff. Read the comment docs.

[dmtrKovalenko]

That’s weird. We neeed to fix a problem but not a symptom. Why it was broken? There is no more way to infer style types?

[mvestergaard]

See the linked issue, changes were made in @material-ui/[email protected]

[mvestergaard]

This is blocking upgrade to material-ui 4.9.0 so I'm just suggesting this small fix to make them work together.
Otherwise maybe @eps1lon can weigh in with how it's intended to work with the changes made to types in 4.9.0

[dmtrKovalenko]

Let’s wait a bit, I will investigate this issue when be at office. 1-2 hours

[mvestergaard]

An alternative may be to change it to WithStyles<typeof styles, true> or WithStyles<typeof styles, false> same as in Calendar. That overload doesn't appear to cause an error. What do you think?

[eps1lon]

Otherwise maybe @eps1lon can weigh in with how it's intended to work with the changes made to types in 4.9.0

It shouldn't unless your types were unsound to begin with. We had one union member that wasn't used in our test suite though. So this might help finding a test that makes use of that particular member.

I'll probably won't have time looking into this particular issue this week. Though I am working on the picker types currently so this might include a useful side-effect.

[dmtrKovalenko]

Yeah let's fix it for now with working overload. Thanks.

[mvestergaard]

My suggested alternative doesn't work after all, the reason it works on Calendar and not Clock is that on Calendar the styles are defined as:

export const styles = (theme: Theme) => ({
  transitionContainer: {
...

while on Clock:

export const styles = (theme: Theme) =>
  createStyles({
    container: {
...

I'm not sure if there are any other consequences from removing createStyles than loosing intellisense on the style definition.

[mvestergaard]

Removing createStyles results in all sorts of other build issues.
Seems like the original proposed change on this PR is the smallest change needed to make this work.
Can we go with that for now, and at least make it work, then work on making it "correct" later?

[mvestergaard]

I've updated the approach to infer the ClassKey using keyof, this should at least be a middle ground you can tolerate?

[eps1lon]

So yeah we're encountering this as well. Something else is going on though. It seems like it's using an outdated signature of createStyles.

Edit: Pretty sure this is only an issue because typescript inlined the type. It expanded StyleRules to CSSProperties | () => CSSProperties. In 4.9 we changed the signature of StyleRules to just CSSProperties but since typescript inlined the older on this change wasn't propagated.

We just need to bump the used version of core in this repository and republish.

I would consider this a bug in typescript though. /core is a peer and therefore no types should be inlined.

[dmtrKovalenko]

I fixed the root of this issue and released v2.3.10, so this can be closed.
But anyway, big thanks to @mvestergaard for quick contribution 💪