This seems unrelated?

@syranide
No. Some tests depend on this issue's behavior.

https://github.com/facebook/react/blob/master/src/test/__tests__/ReactTestUtils-test.js#L433-L465

I prefer that the SyntheticEvent in TestUtils.Simulate isn't released after the event callback has been invoked.
Should I separate a commit or create an another PR?

I don't know enough about testing and how this is used, but if it affects behavior for the event received by the callbacks then this seems wrong.

We shouldn't do this. It would make Simulate.* event handlers behave differently than they would in the browser where events aren't automatically persisted (eg a handler calls something with the event in a timeout would end up working in tests but fail in real life).

We should fix that and any other tests, something like this.

var foo = {
  onChange = function(e) {
    e.persist();
  }
}
spyOn(foo, 'onChange').andCallThrough();

Thanks. I removed this line and fixed the failed tests!

We probably want to do this for currentTarget as well.

@zpao I think currentTarget is already nullifiled because it is in the EventInterface property.

• https://github.com/facebook/react/blob/master/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js#L27
• https://github.com/facebook/react/blob/master/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js#L157-L160

Ah yea, good call. Really target should be in that interface too but I get why it's not.

@zpao

Really target should be in that interface too

I think so. DOM3 Event interface has target attribute.
SyntheticEvent has all DOM3 Event interface attributes except target.

https://www.w3.org/TR/DOM-Level-3-Events/#event-interfaces

Why it's not.

Isn't this enough?

diff --git a/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js b/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
index f9d0206..48ba4e8 100644
--- a/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
+++ b/src/renderers/dom/client/syntheticEvents/SyntheticEvent.js
@@ -23,6 +23,7 @@ var warning = require('warning');
  */
 var EventInterface = {
   type: null,
+  target: null,
   // currentTarget is set when dispatching; no use in copying it here
   currentTarget: emptyFunction.thatReturnsNull,
   eventPhase: null,
@@ -55,16 +56,17 @@ var EventInterface = {
 function SyntheticEvent(dispatchConfig, targetInst, nativeEvent, nativeEventTarget) {
   this.dispatchConfig = dispatchConfig;
   this._targetInst = targetInst;
-
   this.nativeEvent = nativeEvent;
-  this.target = nativeEventTarget;
-  this.currentTarget = nativeEventTarget;

   var Interface = this.constructor.Interface;
   for (var propName in Interface) {
     if (!Interface.hasOwnProperty(propName)) {
       continue;
     }
+    if (propName === 'target') {
+      this.target = nativeEventTarget;
+      continue;
+    }
     var normalize = Interface[propName];
     if (normalize) {
       this[propName] = normalize(nativeEvent);
@@ -160,7 +162,6 @@ assign(SyntheticEvent.prototype, {
     this.dispatchConfig = null;
     this._targetInst = null;
     this.nativeEvent = null;
-    this.target = null;
   },

 });

Yea, that looks right. Seems like we shouldn't have been setting currentTarget in the c'tor anyway.

I get why it's not

I just meant that it feels awkward to have to check for === 'target' every time. We could probably move it into the else case below where you have it and then drop the continue.

@zpao Thanks! I updated this.