Only purgeID on ReactDOMComponent unmount

[syranide]

Fixes #1567

[sophiebits]

nit: can you move this above the super call?

[syranide]

Makes sense, will do.

[sophiebits]

Looks good to me.

[syranide]

Technically, the neatest solution would be for ReactTextComponent to use a React.DOM.span internally, but I'm guessing the overhead would be unpreferable.

[sophiebits]

Or perhaps add it to ReactBrowserComponentMixin? Not sure.

[syranide]

Hmm, yeah that's a good point.

I guess the upside of using React.DOM.span in ReactTextComponent would be that only ReactDOMComponent actually generates DOM nodes then (I currently have a special-case for ReactTextComponent, since it doesn't have a tagName). We could also get rid of ReactBrowserComponentMixin then I think?

[sophiebits]

react-art also uses it: https://github.com/facebook/react-art/blob/c719f03/src/ReactART.js#L184-L189.

[syranide]

Right, but the solution would be the same for it then, use React.DOM.* instead.

[syranide]

Related to discussion, #1598

[zpao]

This was the only place using ReactComponentEnvironment.unmountIDFromEnvironment - can we probably get rid of that now.

[syranide]

@zpao Hmm? That's exactly what's going on here? :) (I find no occurences of unmountIDFromEnvironment)

[zpao]

[syranide]

Btw, rebased this too.

Test plan: grunt test and manually interacting with our application

[syranide]

@sebmarkbage Rewrote to fix conflicts, it's still the same basically, but... I chose to use ReactComponentBrowserEnvironment.unmountIDFromEnvironment because that's what ReactDOMTextComponent is using, is that correct or should both be using ReactMount.purgeID directly?

EDIT: I've also ran this PR against our app and I have witnessed no stale nodes in the nodeCache, except for #2988 which is not caused by this PR.

Warning, it seems React's focus/selection restoration it repopulating the cache with the last focused nodes after being purged (when unmounted). I'm not sure if it's happening in stable as well, but it's happening when this PR is applied. Will have to look into that before this can be merged. It's happening in stable as well.