[Popover] Fix wrong getContentAnchorEl definition

[duvet86]

Update typescript definition.

Closes #12539

[eps1lon]

While this is indeed correct I would rather change the warning message. This change could theoretically be applied to most optional props which would add way to much noise in my opinion.

You could also pass 0 or false and suppress the warning. The only difference is that the component would not behave as expected and prop-types would issue a warning. Because prop-types considers optional props to match T | null | undefined. However both flow and typescript consider optional types to be T | undefined.

[oliviertassinari]

@eps1lon Interesting concern, what do you think of
https://github.com/mui-org/material-ui/blob/96d2c38cfa6f5d8800b8a2632bff4cab56fd9472/packages/material-ui/src/Popover/Popover.d.ts#L22
and
https://github.com/mui-org/material-ui/blob/96d2c38cfa6f5d8800b8a2632bff4cab56fd9472/packages/material-ui/src/Popper/Popper.d.ts#L22

What would you suggest as the best path forward?

[eps1lon]

From my experience people are not that interested in semantic versioning considering typings. While this would be a breaking change at compile time the runtime would be unaffected.

Since you just bumped your typescript version to the next major (#12409) I think it's ok to define a consistent typing considering optional types and include that change in the next minor.

[oliviertassinari]

@eps1lon I have to admit, I don't follow here you are going with this. What do you think we merge this pull request so we stay consistent with the other occurrences of null in the TypeScript definition? And then, we work on a global better answer? cc @pelotom

[eps1lon]

That is completely fine by me.

I just have to be a little pedantic here: The warning should also mention null or undefined if we merge this.

[oliviertassinari]

@duvet86 It's a great first pull request on Material-UI 👌🏻. Thank you for giving it a shot!

[pelotom]

I agree with @eps1lon, I would rather change all the TS definitions to exclude null. It’s common practice in TS to only use undefined for “nully” things, because of its compatibility with optional (?) properties. There’s even a tslint rule to forbid the use of null.

[oliviertassinari]

At least, the warning message was updated :).