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

import Transition, { 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 = 0 Transition.EXITED = 1 Transition.ENTERING = 2 Transition.ENTERED = 3 Transition.EXITING = 4

Instead of the exported ones:

export const UNMOUNTED = 'unmounted' export const EXITED = 'exited' export const ENTERING = 'entering' export const ENTERED = 'entered' export const EXITING = '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.

class Test { static get prop() { return "val1"; } } console.log(Test.prop); // val1 Test.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. 🤷‍♂️