events: EventTarget use validators and small fix

[lundibundi]

• events: fix depth in customInspectSymbol and clean up
• events: use internal/validators in event_target.js

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

/cc @jasnell @nodejs/events

[lundibundi]

I'd assume this was a bug (dept instead of depth) but wanted to point out since I'm not sure how this interaction works (inspecting circular events I assume?).
Also, use NumberIsInteger instead of just null check.

[BridgeAR]

That is both correct. The user should only ever pass through null or and integer but it would be possible to pass through other types as well.

[targos]

Suggested change

	Object,
	NumberIsInteger,
	NumberIsInteger,
	Object,

[benjamingr]

Generally LGTM - can you add a test?

[lundibundi]

Generally LGTM - can you add a test?

Do you mean test for inspecting of Event and EventTarget with depth parameter?
I'm not sure how to do that, the Event doesn't have any circular references in customInspectSymbol handler and the EventTarget just inspects empty object (bug possibly?) in customInspectSymbol which is why I was confused by the actual need of the custom inspect there.

[lundibundi]

Also, @BridgeAR shouldn't this return this? (as well as the other one)

[BridgeAR]

Yes, that would be ideal. this will just let inspect() to properly know what to do.

[benjamingr]

Do you mean test for inspecting of Event and EventTarget with depth parameter?

Yes. cc @jasnell wdyt?

If this isn't too much work to test I think we should add a test - but if it's a lot of work I am fine with LGTMing without one.

[lundibundi]

This is now (after force-push) waiting (adds the same code as)
#33611
#33623
#33613

[BridgeAR]

This needs a rebase.

[BridgeAR]

Yes, that would be ideal. this will just let inspect() to properly know what to do.

[BridgeAR]

That is both correct. The user should only ever pass through null or and integer but it would be possible to pass through other types as well.

[BridgeAR]

@lundibundi I rebased your branch and everything should be in order again. The tests pass locally. Please have a look to verify that, thanks.

[lundibundi]

@BridgeAR thanks :).
I wanted to wait until those PRs land to incorporate changes but I guess I can now just comment there instead of waiting.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/31675/

[jasnell]

This needs another rebase, unfortunately.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/31948/

[jasnell]

Closing in favor of #34015 (which combines this and other eventtarget PRs into a single to make landing easier