Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(stash): clean up mutations, emit updates as a list #3376

Merged
merged 18 commits into from
Jan 6, 2025
Merged

feat(stash): clean up mutations, emit updates as a list #3376

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
4b08147
pull out stash changes from entrykit
alvrs Nov 20, 2024
d4cd627
remove unrelated changes
alvrs Nov 20, 2024
0a849d8
fix tests
alvrs Nov 20, 2024
9452fd9
update types
alvrs Nov 27, 2024
b3ec3a4
update registerTable
alvrs Nov 27, 2024
fe4481a
fix type issues
alvrs Nov 27, 2024
368d3f1
fix subscribeStash test
alvrs Nov 27, 2024
8818ee0
fix subscribeTable test
alvrs Nov 27, 2024
3d02aa2
update createStash test
alvrs Dec 5, 2024
6a5be07
update defaultActions test
alvrs Dec 5, 2024
f7525bb
update getTable test
alvrs Dec 5, 2024
be7cd4b
update query type
alvrs Dec 5, 2024
5cc3324
improve query result and params
alvrs Dec 5, 2024
799ba48
fix query subscriber updates
alvrs Dec 6, 2024
e1c6f2e
add test for multiple updates
alvrs Dec 6, 2024
6dff518
Create popular-lies-teach.md
holic Jan 3, 2025
539f105
Apply suggestions from code review
holic Jan 3, 2025
e081b49
Merge remote-tracking branch 'origin/main' into alvrs/stash-improvements
holic Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix tests
  • Loading branch information
@alvrs
alvrs committed Jan 3, 2025
commit 0a849d8ab32125f0a05406e55732a0139cdd86ba
6 changes: 3 additions & 3 deletions packages/stash/src/actions/applyUpdates.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,9 @@ export function applyUpdates({ stash, updates }: ApplyUpdatesArgs): void {
((storeUpdates.records[table.namespaceLabel] ??= {})[table.label] ??= {})[encodedKey] ??= update;
}

// queueMicrotask(() => {
notifySubscribers(stash);
// });
queueMicrotask(() => {
notifySubscribers(stash);
});
Comment on lines +69 to +71
Copy link
Member

@holic holic Jan 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in my entrykit branch, I had removed queuing because it caused something in the live records sync to fail and fallback to the slower sync methods and was unclear why

Suggested change
queueMicrotask(() => {
notifySubscribers(stash);
});
// TODO: wrap in queueMicrotask here once we figure out why it affects sync fallback
notifySubscribers(stash);

do you know if this is an issue here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tested locally with a fresh template and everything seems fine

making a mental note to come back to this if we see weird issues with syncing, esp large chunks of data

}

function notifySubscribers(stash: Stash) {
Expand Down
26 changes: 11 additions & 15 deletions packages/stash/src/actions/subscribeQuery.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe("defineQuery", () => {

beforeEach(() => {
stash = createStash(config);
vi.useFakeTimers({ toFake: ["queueMicrotask"] });

// Add some mock data
const items = ["0xgold", "0xsilver"] as const;
Expand All @@ -45,6 +46,8 @@ describe("defineQuery", () => {
setRecord({ stash, table: Inventory, key: { player: `0x${String(i)}`, item }, value: { amount: i } });
}
}

vi.advanceTimersToNextTimer();
});

it("should return the matching keys and keep it updated", () => {
Expand All @@ -55,23 +58,22 @@ describe("defineQuery", () => {
});

setRecord({ stash, table: Health, key: { player: `0x2` }, value: { health: 2 } });
vi.advanceTimersToNextTimer();

attest(result.keys).snap({
"0x3": { player: "0x3" },
"0x4": { player: "0x4" },
"0x2": { player: "0x2" },
});
attest(result.keys).snap({ "0x3": { player: "0x3" }, "0x4": { player: "0x4" }, "0x2": { player: "0x2" } });
});

it("should notify subscribers when a matching key is updated", () => {
vi.useFakeTimers({ toFake: ["queueMicrotask"] });

let lastUpdate: unknown;
const subscriber = vi.fn((update: QueryUpdate) => (lastUpdate = update));
const result = subscribeQuery({ stash, query: [Matches(Position, { x: 4 }), In(Health)] });
result.subscribe(subscriber);

const result = subscribeQuery({
stash,
query: [Matches(Position, { x: 4 }), In(Health)],
});
result.subscribe(subscriber);
vi.advanceTimersToNextTimer();

expect(subscriber).toBeCalledTimes(0);

setRecord({ stash, table: Position, key: { player: "0x4" }, value: { y: 2 } });
Expand All @@ -95,8 +97,6 @@ describe("defineQuery", () => {
});

it("should notify subscribers when a new key matches", () => {
vi.useFakeTimers({ toFake: ["queueMicrotask"] });

let lastUpdate: unknown;
const subscriber = vi.fn((update: QueryUpdate) => (lastUpdate = update));
const result = subscribeQuery({ stash, query: [In(Position), In(Health)] });
Expand Down Expand Up @@ -126,8 +126,6 @@ describe("defineQuery", () => {
});

it("should notify subscribers when a key doesn't match anymore", () => {
vi.useFakeTimers({ toFake: ["queueMicrotask"] });

let lastUpdate: unknown;
const subscriber = vi.fn((update: QueryUpdate) => (lastUpdate = update));
const result = subscribeQuery({ stash, query: [In(Position), In(Health)] });
Expand Down Expand Up @@ -157,8 +155,6 @@ describe("defineQuery", () => {
});

it("should notify initial subscribers with initial query result", () => {
vi.useFakeTimers({ toFake: ["queueMicrotask"] });

let lastUpdate: unknown;
const subscriber = vi.fn((update: QueryUpdate) => (lastUpdate = update));
subscribeQuery({ stash, query: [In(Position), In(Health)], options: { initialSubscribers: [subscriber] } });
Expand Down