Add test case for getEventTarget

[lidoravitan]

Added Test case for packages/react-dom/src/events/getEventTarget.js

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[luyaor]

Dear lidoravitan,

We are researchers working on identifying redundant development and duplicated pull requests. We have found there is a pull request: #11528 which might be a potentially duplicate to this one. We would really appreciate if you could help us to validate and give us feedback.

Thank you very much for your time!

Sincerely, Luyao

[gaearon]

@FancyCoder0 These comments are creating notification noise on the repository, could I please ask you not to continue this on the React repository? Thanks :-)

[gaearon]

Thanks for the PR! We have plenty of tests that use .target so this doesn't add valuable coverage.

Maybe it might be worth it if we add a test for corner case like this?

react/packages/react-dom/src/events/getEventTarget.js

Lines 20 to 23 in 1047980

	// Normalize SVG <use> element events #4963
	if (target.correspondingUseElement) {
	target = target.correspondingUseElement;
	}

[lidoravitan]

@gaearon Thanks for response! Sure I will change the PR by your comment to cover the corner case. and update soon :)

[lidoravitan]

@gaearon Updated.
I cover the correspondingUseElement corner case, also cover the last case if target is text node (nodeType).

react/packages/react-dom/src/events/getEventTarget.js

Lines 20 to 27 in 1047980

	// Normalize SVG <use> element events #4963
	if (target.correspondingUseElement) {
	target = target.correspondingUseElement;
	}
	// Safari may fire events on text nodes (Node.TEXT_NODE is 3).
	// @see http://www.quirksmode.org/js/events_properties.html
	return target.nodeType === TEXT_NODE ? target.parentNode : target;

I hope to cover the whole file, so we should cover the first Branch,

react/packages/react-dom/src/events/getEventTarget.js

Line 18 in 1047980

let target = nativeEvent.target || window;

But I can't find any behaviour that nativeEvent.target will be undefined or null without a call directly to getEventTarget function and pass nativeElement.target undefined or null.
Please let me know if you can suggest me how to cover this case? call directly to getEventTarget function and test this case or we should modify the target to null without call this function?

I found something related to fake event, but still can't modify the event.target to undefined.

react/packages/react-dom/src/test-utils/ReactTestUtils.js

Lines 46 to 49 in 8227e54

	function simulateNativeEventOnNode(topLevelType, node, fakeNativeEvent) {
	fakeNativeEvent.target = node;
	ReactDOMEventListener.dispatchEvent(topLevelType, fakeNativeEvent);
	}

[lidoravitan]

@gaearon @sophiebits
I hope we can promote this PR soon, because I would be happy to contribute more below :)
thanks a lot!

[lidoravitan]

@gaearon, please can you check this PR ? thanks.

[gaearon]

If I disable correspondingUseElement logic in getEventTarget.js this test still passes. Seems wrong.

[lidoravitan]

@gaearon I agree that was wrong, so i fixed that. please review again.
thanks :)

[lidoravitan]

@gaearon hope you can review this PR again, thanks

[lidoravitan]

@gaearon @sophiebits Please can you take a look, thanks!

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[lidoravitan]

@gaearon is it could be relevant? or close this PR? btw, I fixed this PR by your comment.

[gaearon]

Hey, thanks for PR. Looking at it again, I'm not sure it adds anything in practice that isn't already covered by other tests. So we probably don't need it after all. Sorry for the churn. The SVG case is interesting, but it's clear enough from the source code itself so if we ever decide to remove this special case, the test won't help us much.