- Sponsor
-
Notifications
You must be signed in to change notification settings - Fork 77
refactor: migrate ClientSessionSyncProcessor
’s materializeEvent()
to Effect
#376
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.
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
base: igor/refactor-client-session-sync-processor-push-to-effect
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the ClientSessionSyncProcessor’s materializeEvent implementation to use Effect for improved instrumentability with OpenTelemetry. Key changes include:
- Updating snapshot files to include a new children field for materialize-event.
- Refactoring materializeEvent in store.ts to use Effect.fn and Effect.gen.
- Updating ClientSessionSyncProcessor to yield* the new materializeEvent effect.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/@livestore/react/src/snapshots/useClientDocument.test.tsx.snap | Updated snapshots with the new children property for materialize-event. |
packages/@livestore/livestore/src/store/store.ts | Transformed materializeEvent to use Effect, with changes to error handling and asynchronous behavior. |
packages/@livestore/livestore/src/live-queries/snapshots/db-query.test.ts.snap | Updated snapshots to include new children for materialize-event. |
packages/@livestore/common/src/sync/ClientSessionSyncProcessor.ts | Adjusted the materializeEvent signature and added yield* calls to invoke the new effect. |
Comments suppressed due to low confidence (1)
packages/@livestore/livestore/src/store/store.ts:155
- Consider wrapping the sqliteDbWrapper.cachedExecute call within a try-catch block inside the Effect.sync callback to preserve the error context from the previous implementation. If relying solely on the effect's error channel, ensure that the absence of a custom error wrapper does not impact debugging or error tracing.
this.sqliteDbWrapper.cachedExecute(statementSql, bindValues, { otelContext, writeTables })
a2bc64d
to
ad77fc1
Compare
1bcfa59
to
2a6b1ad
Compare
ad77fc1
to
6e169e9
Compare
05a562b
to
d9cee66
Compare
6e169e9
to
feb4b6b
Compare
1ec08dd
to
93f3115
Compare
9fff208
to
3512305
Compare
93f3115
to
3e014c0
Compare
3512305
to
42459d2
Compare
42459d2
to
a607146
Compare
3e014c0
to
4ee1579
Compare
a607146
to
a65a1b7
Compare
4ee1579
to
543308e
Compare
1690716
to
ec08957
Compare
0eddddb
to
0defe0e
Compare
38c991b
to
4978123
Compare
02a30c8
to
e42880f
Compare
4978123
to
8b36600
Compare
d5f5100
to
3a2f28a
Compare
8d07a4b
to
462bd0c
Compare
2f1778d
to
1037dfd
Compare
462bd0c
to
c6e6198
Compare
1037dfd
to
feac3a1
Compare
feac3a1
to
7bd8205
Compare
c6e6198
to
20d0e77
Compare
7bd8205
to
75c4aa0
Compare
256ec88
to
67064d4
Compare
75c4aa0
to
1c517d0
Compare
c350558
to
29d458c
Compare
daeb6f0
to
d6321b3
Compare
…nc-processor:materialize-event" span
d6321b3
to
9644d7c
Compare
This migrates
ClientSessionSyncProcessor
’smaterializeEvent()
to Effect to make it easier to instrument it with OpenTelemetry.Caution
DO NOT MERGE!
This is a stacked PR. To preserve logical order, and a clean commit history, these must be merged from the bottom upwards. I'll handle the merges and rebases myself, as it can be tricky to do properly.
Merge order:
ClientSessionSyncProcessor.push()
to Effect #375 (base)ClientSessionSyncProcessor
’smaterializeEvent()
to Effect #376store.commit
to Effect #381 (top)Checklist
non-exclusive, royalty-free, irrevocable copyright license to reproduce, prepare
derivative works of, publicly display, sublicense, and distribute this
Contribution and such derivative works.
Contribution contains no content requiring a license from any third party.