diff --git a/mesop/components/input/input.ts b/mesop/components/input/input.ts index a41c1b60..070b56a2 100644 --- a/mesop/components/input/input.ts +++ b/mesop/components/input/input.ts @@ -10,8 +10,6 @@ import { import {InputType} from 'mesop/mesop/components/input/input_jspb_proto_pb/mesop/components/input/input_pb'; import {Channel} from '../../web/src/services/channel'; import {formatStyle} from '../../web/src/utils/styles'; -import {Subject} from 'rxjs'; -import {debounceTime} from 'rxjs/operators'; import {CommonModule} from '@angular/common'; @Component({ @@ -24,22 +22,8 @@ export class InputComponent { @Input() key!: Key; @Input() style!: Style; private _config!: InputType; - private inputSubject = new Subject(); - constructor(private readonly channel: Channel) { - this.inputSubject - .pipe( - // Setting this to a short duration to avoid having the user trigger another event - // during this debounce time period: - // https://github.com/google/mesop/issues/171 - debounceTime(150), - ) - .subscribe((event) => this.onInputDebounced(event)); - } - - ngOnDestroy(): void { - this.inputSubject.unsubscribe(); - } + constructor(private readonly channel: Channel) {} ngOnChanges() { this._config = InputType.deserializeBinary( @@ -72,14 +56,11 @@ export class InputComponent { } onInput(event: Event): void { - this.inputSubject.next(event); - } - - onInputDebounced(event: Event): void { const userEvent = new UserEvent(); userEvent.setHandlerId(this.config().getOnInputHandlerId()!); userEvent.setStringValue((event.target as HTMLInputElement).value); userEvent.setKey(this.key); + userEvent.setMergeable(true); this.channel.dispatch(userEvent); } } diff --git a/mesop/examples/__init__.py b/mesop/examples/__init__.py index 90d4a48c..bc728aa6 100644 --- a/mesop/examples/__init__.py +++ b/mesop/examples/__init__.py @@ -11,6 +11,7 @@ from mesop.examples import generator as generator from mesop.examples import grid as grid from mesop.examples import index as index +from mesop.examples import input_race as input_race from mesop.examples import integrations as integrations from mesop.examples import llm_rewriter as llm_rewriter from mesop.examples import many_checkboxes as many_checkboxes diff --git a/mesop/examples/input_race.py b/mesop/examples/input_race.py new file mode 100644 index 00000000..62dea14d --- /dev/null +++ b/mesop/examples/input_race.py @@ -0,0 +1,32 @@ +import mesop as me + + +@me.page(path="/input_race") +def app(): + state = me.state(State) + me.input(key="a", label="Input a", on_input=on_input) + me.input(key="b", label="Input b", on_input=on_input) + me.button(label="Click", on_click=on_click) + me.text("State.input_a: " + state.input_a) + me.text("State.input_b: " + state.input_b) + me.text("State.input_on_click: " + state.input_on_click) + + +@me.stateclass +class State: + input_a: str + input_b: str + input_on_click: str + + +def on_input(e: me.InputEvent): + state = me.state(State) + if e.key == "a": + state.input_a = e.value + elif e.key == "b": + state.input_b = e.value + + +def on_click(e: me.ClickEvent): + state = me.state(State) + state.input_on_click = state.input_a diff --git a/mesop/protos/ui.proto b/mesop/protos/ui.proto index 70a8692c..03b16fe6 100644 --- a/mesop/protos/ui.proto +++ b/mesop/protos/ui.proto @@ -25,6 +25,12 @@ message UserEvent { optional Key key = 3; + // If a UserEvent is mergeable, this means that multiple user events + // from the same source (i.e. handler id and key) fired in + // quick succession can be turned into a single user event to improve + // performance. + optional bool mergeable = 10; + oneof type { bool bool_value = 4; string string_value = 5; diff --git a/mesop/server/server.py b/mesop/server/server.py index 2fb6fc7c..d196df85 100644 --- a/mesop/server/server.py +++ b/mesop/server/server.py @@ -36,7 +36,6 @@ def render_loop( ) -> Generator[str, None, None]: try: runtime().run_path(path=path, trace_mode=trace_mode) - title = runtime().get_path_title(path=path) root_component = runtime().context().current_node() diff --git a/mesop/tests/e2e/input_race_test.ts b/mesop/tests/e2e/input_race_test.ts new file mode 100644 index 00000000..e419c4d2 --- /dev/null +++ b/mesop/tests/e2e/input_race_test.ts @@ -0,0 +1,29 @@ +import {test, expect} from '@playwright/test'; + +test('test', async ({page}) => { + // This test is relatively slower because there's a few + // sequential steps. + test.setTimeout(30000); + + await page.goto('/input_race'); + + // Trigger an initial user event. + await page.getByLabel('Input a').fill('1'); + // Trigger two user events "2", "3" which will + // probably get queued. + await page.getByLabel('Input a').pressSequentially('23'); + // Trigger a user event with the same handler function + // but different key to check that this edge case is handled. + await page.getByLabel('Input b').fill('45'); + // Trigger a click user event to save the value of Input a. + await page.getByRole('button', {name: 'Click'}).click(); + + // Assert that the text for each field is as expected. + await expectTextExists('State.input_a: 123'); + await expectTextExists('State.input_b: 45'); + await expectTextExists('State.input_on_click: 123'); + + async function expectTextExists(text: string) { + expect(await page.getByText(text).textContent()).toEqual(text); + } +}); diff --git a/mesop/web/src/services/channel.ts b/mesop/web/src/services/channel.ts index 6d51c156..b8a48d5a 100644 --- a/mesop/web/src/services/channel.ts +++ b/mesop/web/src/services/channel.ts @@ -46,6 +46,7 @@ export class Channel { private status!: ChannelStatus; private componentConfigs: readonly ComponentConfig[] = []; private queuedEvents: (() => void)[] = []; + queuedEventsByHandlerKey: Map void)[]> = new Map(); constructor( private logger: Logger, @@ -157,25 +158,61 @@ export class Channel { } dispatch(userEvent: UserEvent) { + const handlerId = userEvent.getHandlerId(); + const handlerKey = `handlerId=${handlerId}|key=${userEvent.getKey()}`; // Except for navigation user event, every user event should have // an event handler. - if (!userEvent.getHandlerId() && !userEvent.getNavigation()) { + if (!handlerId && !userEvent.getNavigation()) { // This is a no-op user event, so we don't send it. return; } + const initUserEvent = () => { userEvent.setStates(this.states); const request = new UiRequest(); request.setUserEvent(userEvent); this.init(this.initParams, request); + + // Early return if this isn't a mergeable user event. + if (!handlerId || !userEvent.getMergeable()) { + return; + } + + // Garbage collect from this.queuedEventsByHandlerKey + // to avoid memory leak. + const queuedEvents = this.queuedEventsByHandlerKey.get(handlerKey); + const foundEventIndex = queuedEvents!.findIndex( + (e) => e === initUserEvent, + ); + if (foundEventIndex === -1) { + console.error('Could not clean up user event', initUserEvent); + } + queuedEvents!.splice(foundEventIndex, 1); }; + + // This merges existing queued events from the same source + // (handler id + component key) into the current user event + // by removing existing queued events from the same source. + if (handlerId && userEvent.getMergeable()) { + if (!this.queuedEventsByHandlerKey.get(handlerKey)) { + this.queuedEventsByHandlerKey.set(handlerKey, []); + } + const existingEvents = this.queuedEventsByHandlerKey.get(handlerKey); + const remainingEvents = existingEvents!.filter( + (event) => !this.queuedEvents.includes(event), + ); + this.queuedEvents = this.queuedEvents.filter( + (event) => !existingEvents!.includes(event), + ); + this.queuedEventsByHandlerKey.set(handlerKey, remainingEvents); + this.queuedEventsByHandlerKey.get(handlerKey)!.push(initUserEvent); + } + this.logger.log({type: 'UserEventLog', userEvent: userEvent}); if (this.status === ChannelStatus.CLOSED) { initUserEvent(); } else { - this.queuedEvents.push(() => { - initUserEvent(); - }); + this.queuedEvents.push(initUserEvent); } }