Skip to content

Commit

Permalink
Replace input debouncing with a more robust mechanism for de-duplicat…
Browse files Browse the repository at this point in the history
…ing queued user events
  • Loading branch information
wwwillchen committed May 11, 2024
1 parent b4944ca commit 39cf4b3
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 26 deletions.
23 changes: 2 additions & 21 deletions mesop/components/input/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -24,22 +22,8 @@ export class InputComponent {
@Input() key!: Key;
@Input() style!: Style;
private _config!: InputType;
private inputSubject = new Subject<Event>();

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(
Expand Down Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions mesop/examples/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 32 additions & 0 deletions mesop/examples/input_race.py
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions mesop/protos/ui.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion mesop/server/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
29 changes: 29 additions & 0 deletions mesop/tests/e2e/input_race_test.ts
Original file line number Diff line number Diff line change
@@ -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);
}
});
45 changes: 41 additions & 4 deletions mesop/web/src/services/channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class Channel {
private status!: ChannelStatus;
private componentConfigs: readonly ComponentConfig[] = [];
private queuedEvents: (() => void)[] = [];
queuedEventsByHandlerKey: Map<string, (() => void)[]> = new Map();

constructor(
private logger: Logger,
Expand Down Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 39cf4b3

Please sign in to comment.