-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[@xstate/store] Refactor store logic to improve context updates and snapshot handling #5323
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: main
Are you sure you want to change the base?
Changes from all commits
56a5717
f10f659
5eda50b
11de5d4
ff0385b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
--- | ||
'@xstate/store': minor | ||
--- | ||
|
||
Added support for effect-only transitions that don't trigger state updates. Now, when a transition returns the same state but includes effects, subscribers won't be notified of a state change, but the effects will still be executed. This helps prevent unnecessary re-renders while maintaining side effect functionality. | ||
|
||
```ts | ||
it('should not trigger update if the snapshot is the same even if there are effects', () => { | ||
const store = createStore({ | ||
context: { count: 0 }, | ||
on: { | ||
doNothing: (ctx, _, enq) => { | ||
enq.effect(() => { | ||
// … | ||
}); | ||
return ctx; // Context is the same, so no update is triggered | ||
// This is the same as not returning anything (void) | ||
} | ||
} | ||
}); | ||
|
||
const spy = vi.fn(); | ||
store.subscribe(spy); | ||
|
||
store.trigger.doNothing(); | ||
store.trigger.doNothing(); | ||
|
||
expect(spy).toHaveBeenCalledTimes(0); | ||
}); | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -447,6 +447,43 @@ it('effects can be enqueued', async () => { | |
expect(store.getSnapshot().context.count).toEqual(0); | ||
}); | ||
|
||
it('effect-only transitions should execute effects', () => { | ||
const spy = vi.fn(); | ||
const store = createStore({ | ||
context: { count: 0 }, | ||
on: { | ||
justEffect: (ctx, _, enq) => { | ||
enq.effect(spy); | ||
} | ||
} | ||
}); | ||
|
||
store.trigger.justEffect(); | ||
|
||
expect(spy).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
it('emits-only transitions should emit events', () => { | ||
const spy = vi.fn(); | ||
const store = createStore({ | ||
context: { count: 0 }, | ||
emits: { | ||
emitted: () => {} | ||
}, | ||
on: { | ||
justEmit: (ctx, _, enq) => { | ||
enq.emit.emitted(); | ||
} | ||
} | ||
}); | ||
|
||
store.on('emitted', spy); | ||
|
||
store.trigger.justEmit(); | ||
|
||
expect(spy).toHaveBeenCalledTimes(1); | ||
}); | ||
|
||
describe('store.trigger', () => { | ||
it('should allow triggering events with a fluent API', () => { | ||
const store = createStore({ | ||
|
@@ -766,6 +803,45 @@ it('can be created with a logic object', () => { | |
store.getSnapshot().context.count satisfies string; | ||
}); | ||
|
||
it('should not trigger update if the snapshot is the same', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note this diverges from XState too: import { createActor, createMachine } from "xstate";
const m = createMachine({
on: {
FOO: {
actions: () => {},
},
},
});
const a = createActor(m).start();
let s = a.getSnapshot();
a.subscribe((_s) => {
console.log("next", s === _s, _s);
s = _s;
});
a.send({ type: "FOO" });
a.send({ type: "FOO" });
a.send({ type: "FOO" }); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about this behavior? If only actions/effects occur (invisible to the state, at least right now), then the state is going to be exactly the same. But I guess that's the responsibility of the consumer to "filter" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similarly here, I think it would be totally OK to filter those out - but I think it would be great if this could be consistent between XState libraries. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think in XState v6, the purpose of the observer is to observe snapshot changes, so notifying on the same snapshot isn't useful. |
||
const store = createStore({ | ||
context: { count: 0 }, | ||
on: { | ||
doNothing: (ctx) => ctx | ||
} | ||
}); | ||
|
||
const spy = vi.fn(); | ||
store.subscribe(spy); | ||
|
||
store.trigger.doNothing(); | ||
store.trigger.doNothing(); | ||
|
||
expect(spy).toHaveBeenCalledTimes(0); | ||
}); | ||
|
||
it('should not trigger update if the snapshot is the same even if there are effects', () => { | ||
const store = createStore({ | ||
context: { count: 0 }, | ||
on: { | ||
doNothing: (ctx, _, enq) => { | ||
enq.effect(() => { | ||
// … | ||
}); | ||
return ctx; | ||
} | ||
} | ||
}); | ||
|
||
const spy = vi.fn(); | ||
store.subscribe(spy); | ||
|
||
store.trigger.doNothing(); | ||
store.trigger.doNothing(); | ||
|
||
expect(spy).toHaveBeenCalledTimes(0); | ||
}); | ||
|
||
describe('types', () => { | ||
it('AnyStoreConfig', () => { | ||
function transformStoreConfig(_config: AnyStoreConfig): void {} | ||
|
Uh oh!
There was an error while loading. Please reload this page.