fix bug from issue #460

[dimensi]

I found an #460 with a bug, since the bug is simple, I decided to correct the error.
The error was that we did not take into account the case when there is a node and there is no timeout.
I also set the enter and exit values in prop-types as reqiured.

[dimensi]

@jquense ?

[silvenon]

Reviewing your PR now. 👀

[silvenon]

Could you give more details about what your tests are proving? By "at once" do you mean "immediately"? How is time measured here?

I was thinking that actually 0 does make sense as a default timeout value for enter and exit. I think this would be the most logical step forward and wouldn't cause warnings for existing code. What do you think?

If you agree, would you mind reverting the change for prop types and instead of current tests add simpler tests where by measuring time you ensure that:

1. in the absence of enter, the status upon entering immediately becomes entered
2. in the absence of exit, the status upon exiting immediately becomes exited

I don't think we have to test the unmountOnExit prop for this, we just want to ensure the default values.

[silvenon]

Concerning prop types, you can take this opportunity to actually fix them:

PropTypes.oneOfType([
  PropTypes.number,
  PropTypes.shape({
    enter: PropTypes.number,
    exit: PropTypes.number,
    appear: PropTypes.number,
  })
]).isRequired

By moving isRequired to the correct place (at the end of oneOfType instead of shape), people who forget to add the timeout prop will finally get a warning.

I'm honestly not sure what isRequired does within oneOfType 🤔

[silvenon]

Here is the summary of requested changes. Sorry for so many comments, it's a bit confusing because changes themselves are overlapping. 😅

[dimensi]

Could you give more details about what your tests are proving? By "at once" do you mean "immediately"? How is time measured here?

yes, i don't know english, i use google translate for find a word.

[silvenon]

Awesome, thank you! 🏆

[dimensi]

@silvenon thx for your review. Are you sure my tests right writed?

[silvenon]

@dimensi yep, they look good to me. I didn't even think to combine both functionalities in the same test. We might later split that into two tests because they are two different features.

[jquense]

🎉 This PR is included in version 2.6.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀