Fix Transition constants to match

[perrin4869]

Ran into a bug while using react-bootstrap and traced the root cause back here.

react-bootstrap is importing the different constants in their source code as follows:

https://github.com/react-bootstrap/react-bootstrap/blob/d28b4f04acbb6910aac4417480f47f5503275791/src/Collapse.js#L7

importTransition,{EXITED,ENTERED,ENTERING,EXITING,}from'react-transition-group/Transition';

However, their build ends up using the values defined in the bottom of src/Transition.js in this repo:

Transition.UNMOUNTED=0Transition.EXITED=1Transition.ENTERING=2Transition.ENTERED=3Transition.EXITING=4

Instead of the exported ones:

exportconstUNMOUNTED='unmounted'exportconstEXITED='exited'exportconstENTERING='entering'exportconstENTERED='entered'exportconstEXITING='exiting'

This PR makes the values match in order to fix the inconsistencies, and should not be a breaking change, since the features would work exactly the same as documented. Maybe the constants Transition.UNMOUNTED, Transition.EXITED, etc, should be removed altogether though, since I am not sure they serve any purpose

[silvenon]

Wow, thanks! Let's keep these constants attached for now, but they definitely need to go at some point.

[perrin4869]

@silvenon thanks for the merge, completely missed it hahaha
Any idea when you will make a release with this fix in?

[silvenon]

After merging #559, which will be soon. 😉

[jquense]

🎉 This PR is included in version 4.4.0 🎉

The release is available on:

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

Your semantic-release bot 📦🚀

[unrevised6419]

@silvenon you mean e.g. Transition.UNMOUNTED should be deprecated?

[silvenon]

I guess, because these constants are already being exported, so this is a duplication.

[unrevised6419]

The problem is that they are not exported directly from react-transition-group.
I made a commit with some changes, you can create a PR from that.

https://github.com/reactjs/react-transition-group/compare/master...iamandrewluca:brent?expand=1

[perrin4869]

Hm... Would still need to figure out why the cjs build from react-bootstrap ends up using the Transition statics instead of the named exports: https://unpkg.com/browse/react-bootstrap@1.0.1/cjs/Collapse.js

[unrevised6419]

@perrin4869 Because they are not exported from react-transition-group. They are not part of the public API.

Not recommended, this is not public API

import{ENTERING}from'react-transition-group/Transition'

Recommended (but not exported right now)

import{ENTERING}from'react-transition-group'

[perrin4869]

@iamandrewluca hm... with your change, I think cjs builds will always warn.

classTest{staticgetprop(){return"val1";}}console.log(Test.prop);// val1Test.prop="val2";console.log(Test.prop);// still val1

cjs builds seem to always refer to Test.prop, which will never be the named export, if there is a static prop defined.

[perrin4869]

This may really be a babel or rollup related issue though

[unrevised6419]

@perrin4869 I don't understand what you want to say exactly.
The warning is showed only once for a unique message. And also will be only showed when e.g. Transition.ENTERED is used.

Also as I see on library build, Transition proxy directory is created. So react-transition-group/Transition it seems to be made on purpose.

[silvenon]

Now that I have time to see what this is about, what specific bug in react-bootstrap did you run into?

Hm... Would still need to figure out why the cjs build from react-bootstrap ends up using the Transition statics instead of the named exports: https://unpkg.com/browse/react-bootstrap@1.0.1/cjs/Collapse.js

I think it is using the exported ones, e.g. Transition.ENTERING is the exported one, Transition.default.ENTERING is the attached one, and I can see in Collapse.js that it's using the exported one.

I don't see a reason for making these constants a part of public API, I just wanted to delete the attached constants because they give the impression that they are indeed public, but that's a wrong way to expose them, if we wanted to do that.

But I don't think there's point in overthinking this, we're probably going to rewrite the component API altogether anyway. 🤷‍♂️