[transition] Allow more accurate PropTypes

[apieceofbart]

As discussed here: #8785 setting children and in props as optional should be brought back to allow using transitions as a props in other components e.g. Snackbar.

I did not add any new tests, I believe there are no tests for checking types.

Breaking change

This breaking change increases the consistency of our API. Also, it's allowing a more strict typing definition that can help spotting issues. One important thing to notice is that the component creation needs to be outside of the render method. Otherwise, a new instance will be created and the animation will lose his state.

-    <Dialog transition={<Slide direction="left" />} />;
+    const Transition = props => <Slide direction="left" {...props} />
+    <Dialog transition={Transition} />;

-    <Snackbar transition={<Slide direction="left" />} />;
+    const Transition = props => <Slide direction="left" {...props} />
+    <Snackbar transition={Transition} />;

[oliviertassinari]

Do we need the same change for flow?

[apieceofbart]

probably yes. I never worked with flow but..

https://github.com/callemall/material-ui/blob/v1-beta/src/internal/transition.js - there are no TransitionProps here.

So I'm guessing you mean type information in concrete transitions like here:
https://github.com/callemall/material-ui/blob/v1-beta/src/transitions/Slide.js#L78

The only problem I see is this line: https://github.com/callemall/material-ui/blob/v1-beta/src/transitions/Grow.js#L200
For some reason Grow gets style from children element which now would be optional. Do you know why is this needed? Other transitions don't extend style prop of children.

[oliviertassinari]

@apieceofbart I'm continuing the PR. I'm gonna simplify the current situation by only accepting a component. This should make things more consistent with the other components and be more accurate.

[apieceofbart]

Sure, go ahead. Let me know if I can help

[oliviertassinari]

@apieceofbart I hope this change address your issue too.

[apieceofbart]

@oliviertassinari thanks for the change but I think this PR didn't include the changes in props typings - namely TransitionProps require children and in props: https://github.com/callemall/material-ui/blob/3e9da458ee69788de85093672f7a334dd80ef641/src/internal/transition.d.ts#L16 while all the examples show self-closing components (so children is missing)

[oliviertassinari]

while all the examples show self-closing components (so children is missing)

@apieceofbart Sounds like an issue on the examples side. Where have you seen them? The children and in property can be provided by the parent.

[apieceofbart]

https://material-ui.com/demos/snackbars/ in the Transition section:

function TransitionLeft(props) {
  return <Slide direction="left" {...props} />;
}

function TransitionUp(props) {
  return <Slide direction="up" {...props} />;
}

function TransitionRight(props) {
  return <Slide direction="right" {...props} />;
}

function TransitionDown(props) {
  return <Slide direction="down" {...props} />;
}

[oliviertassinari]

@apieceofbart This looks good to me. The parent should be injecting the in and children property.

[apieceofbart]

hmmm but Slide (or any other Transition components) requires children:
https://github.com/callemall/material-ui/blob/3e9da458ee69788de85093672f7a334dd80ef641/src/transitions/Slide.js#L78

Those examples clearly don't pass children, right?

[apieceofbart]

If you ask me the best solution would be to leave adding transition to developer. I would then pass my component wrapper with Transition as children to Snackbar. I'm not sure how will that scale to other use cases though.

[oliviertassinari]

Those examples clearly don't pass children, right?

The Snackbar component do pass the children property. I don't get where is the issue. Do you have a codesandbox I can have a look at?

[apieceofbart]

The Snackbar does but the Slide don't. Here you are returning a Slide component with missing children:
return <Slide direction="left" {...props} />;
Maybe this works somehow but Typescript won't compile this code and I guess flow will also have issues.
I will create an example repo.

[oliviertassinari]

The children is a property like another. It will be spread.

[apieceofbart]

Yeah you're right, sorry for that! I was working with wrong example without {...props}.