Skip to content

Commit e32155e

Browse files
authored
fix: cannot update old message after rendering a new one (#275)
* fix: add test to reproduce the error * fix: cannot update old message
1 parent 023e5ed commit e32155e

File tree

22 files changed

+444
-53
lines changed

22 files changed

+444
-53
lines changed

.github/workflows/create-release.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name: Create a new release
33

44
jobs:
55
create-release:
6-
runs-on: ubuntu-latest
6+
runs-on: ubuntu-22.04
77
permissions:
88
contents: write
99
if: ${{ (github.event.pusher.name != 'github action') && (github.ref == 'refs/heads/main') }}

.github/workflows/main.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ name: Test and Lint
33

44
jobs:
55
test:
6-
runs-on: ubuntu-latest
6+
runs-on: ubuntu-22.04
77
steps:
88
- name: Checkout
99
uses: actions/checkout@v4

.github/workflows/released.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ on:
77
jobs:
88
release-npm:
99
name: Release npm packages
10-
runs-on: ubuntu-latest
10+
runs-on: ubuntu-22.04
1111
permissions:
1212
contents: write
1313
steps:

apps/example-bot/src/app/state/counter/counter.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { CommandButton } from "@rx-lab/core";
12
import { useState } from "@rx-lab/storage";
23

34
export function Counter() {
@@ -37,6 +38,9 @@ export function Counter() {
3738
>
3839
Reset
3940
</button>
41+
<CommandButton command={"/"} renderNewMessage>
42+
Home
43+
</CommandButton>
4044
</div>
4145
</div>
4246
);

packages/common/src/adapter.interface.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import {
66
StoredRoute,
77
} from "./router.interface";
88

9+
export type GetRouteKeyLevel = "message" | "chatroom" | "auto";
10+
911
export interface RedirectOptions {
1012
/**
1113
* Indicates whether the application should re-render after the redirect.
@@ -209,12 +211,12 @@ export interface AdapterInterface<
209211
*
210212
* @param container The container object representing the current state of the UI.
211213
* @param isUpdate Indicates whether this is an update to an existing UI or a new render.
212-
* @returns A promise that resolves to the adapted UI element.
214+
* @returns A promise that resolves to the adapted UI element. If the element is not rendered, return undefined.
213215
*
214216
* @example
215217
* const uiElement = await adapter.adapt(container, false);
216218
*/
217-
adapt: (container: C, isUpdate: boolean) => Promise<AdaptElement>;
219+
adapt: (container: C, isUpdate: boolean) => Promise<C | undefined>;
218220

219221
/**
220222
* Sets up the menu structure for the target platform.
@@ -269,7 +271,7 @@ export interface AdapterInterface<
269271
* @param container - The container object holding message context
270272
* @returns A unique string key identifying the route for the current message
271273
*/
272-
getRouteKey: (container: C) => string;
274+
getRouteKey: (container: C, level?: GetRouteKeyLevel) => string;
273275

274276
/**
275277
* Lifecycle method called when core is destroyed.

packages/core/src/core/core.tsx

Lines changed: 57 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
6262
extends Renderer<T>
6363
implements CoreInterface<any>
6464
{
65-
private element: RenderedComponent | undefined;
6665
/**
6766
* Debounce timeout for commit updates.
6867
*/
@@ -149,7 +148,7 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
149148
});
150149

151150
const key = this.adapter.getRouteKey(container);
152-
const storedRoute = await this.storage.restoreRoute(key);
151+
const storedRoute = await this.router.getRouteFromKey(key);
153152
// if user click on a bot message, the message text itself will be erased.
154153
// we need to restore the text from the stored route from user.
155154
if (
@@ -202,7 +201,7 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
202201
});
203202
// get the current route
204203
const key = this.adapter.getRouteKey(container);
205-
const storedRoute = await this.storage.restoreRoute(key);
204+
const storedRoute = await this.router.getRouteFromKey(key);
206205
if (storedRoute) {
207206
await this.redirect(container, storedRoute, {
208207
shouldRender: true,
@@ -301,6 +300,29 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
301300
let component: RenderedComponent | undefined;
302301
if (options?.shouldRender) {
303302
component = await this.loadAndRenderStoredRoute(key, route);
303+
this.once("onRendered", async (renderedContainer) => {
304+
const key = this.adapter.getRouteKey(renderedContainer, "message");
305+
// means this is newly sent message
306+
// we didn't store the route yet
307+
if (options?.shouldAddToHistory) {
308+
Logger.log("Adding initial message to history", "bgBlue");
309+
await Promise.all([
310+
this.storage.saveRoute(
311+
key,
312+
route ?? {
313+
route: DEFAULT_ROOT_ROUTE,
314+
type: "page",
315+
},
316+
),
317+
this.storage.addHistory(key, {
318+
route: route?.route ?? DEFAULT_ROOT_ROUTE,
319+
props: this.renderedPageProps,
320+
type: "page",
321+
}),
322+
]);
323+
}
324+
});
325+
304326
try {
305327
await this.render(
306328
container,
@@ -310,11 +332,18 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
310332
// Important: Do not store the route if the page is redirecting, as redirected pages already have a stored route.
311333
if (component?.isError !== true) {
312334
if (options.shouldAddToHistory) {
313-
await this.storage.saveRoute(key, {
314-
route: route?.route ?? DEFAULT_ROOT_ROUTE,
315-
props: this.renderedPageProps,
316-
type: "page",
317-
});
335+
const globalKey = this.adapter.getRouteKey(container, "chatroom");
336+
337+
// save the message level route and the chatroom level route
338+
await Promise.all(
339+
Array.from(new Set([key, globalKey])).map(async (key) =>
340+
this.storage.saveRoute(key, {
341+
route: route?.route ?? DEFAULT_ROOT_ROUTE,
342+
props: this.renderedPageProps,
343+
type: "page",
344+
}),
345+
),
346+
);
318347
}
319348

320349
if (options?.shouldAddToHistory && route) {
@@ -355,14 +384,24 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
355384
if (!route) return;
356385
// handles the cases while not rendering but still want to add the route to the history
357386
if (options?.shouldAddToHistory) {
358-
await Promise.all([
359-
this.storage.saveRoute(key, route),
360-
this.storage.addHistory(key, {
361-
route: route.route,
362-
props: this.renderedPageProps,
363-
type: "page",
364-
}),
365-
]);
387+
const globalKey = this.adapter.getRouteKey(container, "chatroom");
388+
await Promise.all(
389+
[
390+
this.storage.addHistory(key, {
391+
route: route.route,
392+
props: this.renderedPageProps,
393+
type: "page",
394+
}),
395+
].concat(
396+
Array.from(new Set([key, globalKey])).map(async (key) =>
397+
this.storage.saveRoute(key, {
398+
route: route?.route ?? DEFAULT_ROOT_ROUTE,
399+
props: this.renderedPageProps,
400+
type: "page",
401+
}),
402+
),
403+
),
404+
);
366405
}
367406
}
368407
}
@@ -429,6 +468,8 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
429468
isInGroup: container.isInGroup,
430469
groupId: container.groupId,
431470
hasBeenMentioned: container.hasBeenMentioned,
471+
...this.element.props,
472+
...oldProps,
432473
storage: {
433474
saveState: async (key: string, state: any, opt?: SetStateOptions) => {
434475
const storedKey = encodeStateKey(
@@ -458,8 +499,6 @@ export class Core<T extends Container<BaseChatroomInfo, BaseMessage>>
458499
);
459500
},
460501
},
461-
...this.element.props,
462-
...oldProps,
463502
};
464503

465504
try {

packages/core/src/core/renderer.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
InstanceType,
88
Logger,
99
type ReactInstanceType,
10+
RenderedComponent,
1011
StorageInterface,
1112
} from "@rx-lab/common";
1213
import { Router } from "@rx-lab/router";
@@ -43,6 +44,7 @@ type ServerEvents = {
4344
) => void;
4445
startRenderCallback: () => void;
4546
endRenderCallback: () => void;
47+
onRendered: (container: Container<BaseChatroomInfo, BaseMessage>) => void;
4648
};
4749

4850
export class Renderer<
@@ -81,6 +83,8 @@ export class Renderer<
8183
*/
8284
private currentHasUpdates = false;
8385

86+
protected element: RenderedComponent | undefined;
87+
8488
constructor({ adapter, storage }: RendererOptions) {
8589
super();
8690
const builder = new ComponentBuilder();
@@ -251,7 +255,11 @@ export class Renderer<
251255
"bgCyan",
252256
);
253257
this.currentHasUpdates = false;
254-
await this.adapter.adapt(container, true);
258+
const adaptedContainer = await this.adapter.adapt(container, true);
259+
if (adaptedContainer) {
260+
Logger.log("onRendered event emitted", "bgCyan");
261+
this.emit("onRendered", adaptedContainer);
262+
}
255263
}
256264

257265
async onUpdate(container: T) {

packages/storage/src/memory/memory.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,9 @@ export class MemoryStorage extends Storage {
6363
}
6464

6565
async saveRoute(key: string, path: StoredRoute): Promise<void> {
66-
this.routeMap.set(`${ROUTE_KEY}-${key}`, path);
66+
this.routeMap.set(`${ROUTE_KEY}-${key}`, {
67+
...path,
68+
});
6769
if (this.delaySimulation) {
6870
await this.delaySimulation();
6971
}
@@ -75,7 +77,7 @@ export class MemoryStorage extends Storage {
7577
if (this.delaySimulation) {
7678
await this.delaySimulation();
7779
}
78-
this.historyMap.set(key, route);
80+
this.historyMap.set(key, JSON.stringify(route) as any);
7981
}
8082

8183
async deleteHistory(key: string): Promise<void> {
@@ -89,6 +91,7 @@ export class MemoryStorage extends Storage {
8991
if (this.delaySimulation) {
9092
await this.delaySimulation();
9193
}
92-
return this.historyMap.get(key);
94+
const value = this.historyMap.get(key);
95+
return value !== undefined ? JSON.parse(value as any) : undefined;
9396
}
9497
}

packages/telegram-adapter/src/adapter.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,12 @@ describe("TelegramAdapter", () => {
3737

3838
const result = await adapter.adapt(container as any, false);
3939

40-
expect(result).toEqual([]);
40+
expect(result).toEqual(undefined);
4141
expect(mockBot.sendMessage).not.toHaveBeenCalled();
4242
expect(mockBot.editMessageText).not.toHaveBeenCalled();
4343
});
4444

45-
it("should not send a message if there are no children", async () => {
45+
it.only("should not send a message if there are no children", async () => {
4646
const container: TGContainer = {
4747
type: "ROOT",
4848
children: [],
@@ -55,7 +55,7 @@ describe("TelegramAdapter", () => {
5555

5656
const result = await adapter.adapt(container as any, false);
5757

58-
expect(result).toEqual([]);
58+
expect(result).toEqual(undefined);
5959
expect(mockBot.sendMessage).not.toHaveBeenCalled();
6060
expect(mockBot.editMessageText).not.toHaveBeenCalled();
6161
});

0 commit comments

Comments
 (0)