refactor(core): Split Dispatcher into base class. #55372
Conversation
tbondwilkinson
force-pushed
the
jsaction-dispatcher
branch
5 times, most recently
from
416192f
to
f0b329b
Compare
|
TESTED=TGP |
tbondwilkinson
requested review from
iteriani,
thePunderWoman and
rahatarmanahmed
and removed request for
iteriani
pullapprove
bot
added
the
requires: TGP
This comment was marked as outdated.
This comment was marked as outdated.
| * element of the DOM event, and invokes the handler associated with the | ||
| * jsaction. | ||
| * | ||
| * @param getHandler A function that knows how to get the handler for a |
There was a problem hiding this comment.
Update jsdoc.
Isn't dispatchDelegate supposed to return a handler? Or does the base dispatcher hand the responsibility of calling the handler to dispatchDelegate?
There was a problem hiding this comment.
Base dispatcher hands the responsibility of calling the handler to dispatchDelegate yeah. Updated the comment.
| {eventReplayer = undefined}: {stopPropagation?: boolean; eventReplayer?: Replayer} = {}, | ||
| ) { | ||
| this.eventReplayer = eventReplayer; | ||
| } |
There was a problem hiding this comment.
nit: can you keep the empty lines between methods?
There was a problem hiding this comment.
Yeah that's odd not sure why all the whitespace got removed.
| * Registers deferred functionality for an EventContract and a Jsaction | ||
| * Dispatcher. | ||
| */ | ||
| export function registerDispatcher(eventContract: UnrenamedEventContract, dispatcher: Dispatcher) { |
There was a problem hiding this comment.
Do we need both this and the registerDispatcher in base_dispatcher.ts?
There was a problem hiding this comment.
Because one does not extend the other, the two registerDispatcher functions are technically distinct due to type differences.
I also think it's nice to have the register function available for whichever file you include.
tbondwilkinson
force-pushed
the
jsaction-dispatcher
branch
from
f0b329b
to
c3c5aae
Compare
This will enable refactoring existing usages of the Dispatcher that use APIs that we do not intend to support, like registerGlobalHandler and registerEventInfoHandlers to use the LegacyDispatcher.
tbondwilkinson
force-pushed
the
jsaction-dispatcher
branch
from
c3c5aae
to
63ddfb8
Compare
tbondwilkinson
added
action: merge
thePunderWoman
added
the
target: minor
tbondwilkinson
added
the
merge: caretaker note
|
Caretaker note: This needs cl/626467219 patched on top of the sync CL. Internal test failures are pre-existing and unrelated. |
|
This PR was merged into the repository by commit 98e8613. |
This will enable refactoring existing usages of the Dispatcher that use APIs that we do not intend to support, like registerGlobalHandler and registerEventInfoHandlers to use the LegacyDispatcher.
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently dispatcher with its non-desirable behavior is in one file.
Issue Number: N/A
What is the new behavior?
Dispatcher is split into base_dispatcher, dispatcher, and legacy_dispatcher. Goal is to finish with dispatcher and legacy_dispatcher.
Does this PR introduce a breaking change?