[docs] Icon TypeScript demos

[goldins]

• I have followed (at least) the PR section of the contributing guide.

• Added tsx files for FontAwesome, Icons, SvgIcons, and SvgMaterialIcons.
• Added fg-loadcss type definition stub
• Replace usages of withStyles with makeStyles in these Icon doc files and removed propTypes
• Had to add a few as to work around cloneElement.

[merceyz]

Please run yarn prettier to fix the formatting issues and yarn docs:typescript:formatted to update the JavasScript version.

Not using propTypes in these new TS demos because TS is sufficient.

propTypes are required for the JavaScript version, which means the TypeScript version needs to have it as well.
makeStyles is preferred, so you can use that instead of withStyles and you wont need the propTypes for that at least

[mui-pr-bot]

No bundle size changes comparing 89687f3...beb6548

Generated by 🚫 dangerJS against beb6548

[goldins]

@merceyz thanks for the quick feedback! I've updated the PR to use makeStyles in place of withStyles and removed propTypes now that classes is not being passed in as a prop.

[oliviertassinari]

@goldins The changes look good to me, thanks!

I have looked at the different patterns we use in the codebase:

1. x99

const useStyles = makeStyles((theme: Theme) =>
  createStyles({
    root: {
      display: 'flex',
      justifyContent: 'center',
      alignItems: 'flex-end',
    },

2. x30

const useStyles = makeStyles((theme: Theme) => ({
  root: {
    display: 'flex',
    justifyContent: 'center',
    alignItems: 'flex-end',
  },

3. x6

const useStyles = makeStyles(theme => ({
  root: {
    display: 'flex',
    justifyContent: 'center',
    alignItems: 'flex-end',
  },

I have normized it down to the case n°1 and only n°1.

[goldins]

@oliviertassinari 👍 cool, thanks!

Why did you decide to use case n°1 vs n°3? makeStyles seems to work fine and not require createStyles. Also explicitly using the Theme interface was unnecessary; TS was able to infer it properly, at least in the files I added.

[oliviertassinari]

@goldins I didn't want to derive too much from the original purpose of this pull request. I haven't considered what's the best solution is, I have only considered what's the most popular one in the codebase. I hope we can more easily move between the options as it's done the same way across the codebase.

[goldins]

@oliviertassinari makes sense, thanks.

[merceyz]

Why did you decide to use case n°1 vs n°3? makeStyles seems to work fine and not require createStyles. Also explicitly using the Theme interface was unnecessary

@goldins This depends on which version of TypeScript is in use and which CSS properties you set, using createStyles and setting the theme prevents any issues around this.

The following snippet does not work in the TypeScript version used by material-ui (v3.2.2).
However, if you remove flexGrow: 'inherit' or add createStyles it works fine, so for consistency, createStyles should be used even if it isn't needed.
It works perfectly fine without setting the theme type though

const useStyles = makeStyles(theme => ({
  root: {
    flexGrow: 'inherit'
    margin: theme.spacing(1)
  }
}));

[goldins]

@merceyz Thanks for the info! I knew this was a problem but thought it was fixed with makeStyles (admittedly, I haven't updated my applications to MUI 4 yet). I didn't realize it depends on the specific CSS properties involved (and the Icons files happened to not use any of the ones that would cause the TS error).

Thanks again for your time!

[merceyz]

thought it was fixed with makeStyles

@goldins It is, but it also needs TypeScript v3.4.x

[oliviertassinari]

@goldins Thanks!