Skip to content

Commit

Permalink
Remove observable replay (#3877)
Browse files Browse the repository at this point in the history
* Make actor observable hot

* Infinite loop pissue

* Sorta fix infinite loop test

* Remove test that is no longer valid

* [v5] Tweak changes related to dropping replaying of the last notifications (#3898)

* Tweak changes related to dropping replaying of the last notifications

* Remove unused import

* Fixed Vue test

* remove debugger

* remove unused import

* Skip Solid tests that rely on `from`

* Add changeset

---------

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
  • Loading branch information
davidkpiano and Andarist authored Mar 23, 2023
1 parent 3269374 commit 1269470
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 114 deletions.
19 changes: 19 additions & 0 deletions .changeset/lemon-mails-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
'xstate': major
---

Observing an actor via `actor.subscribe(...)` no longer immediately receives the current snapshot. Instead, the current snapshot can be read from `actor.getSnapshot()`, and observers will receive snapshots only when a transition in the actor occurs.

```ts
const actor = interpret(machine);
actor.start();

// Late subscription; will not receive the current snapshot
actor.subscribe((state) => {
// Only called when the actor transitions
console.log(state);
});

// Instead, current snapshot can be read at any time
console.log(actor.getSnapshot());
```
11 changes: 0 additions & 11 deletions packages/core/src/interpreter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,6 @@ export class Interpreter<
public onTransition(listener: SnapshotListener<TBehavior>): this {
const observer = toObserver(listener);
this.observers.add(observer);

// Send current state to listener
if (this.status === ActorStatus.Running) {
observer.next?.(this.getSnapshot());
}

return this;
}

Expand All @@ -259,11 +253,6 @@ export class Interpreter<

this.observers.add(observer);

// Send current state to listener
if (this.status !== ActorStatus.NotStarted) {
observer.next?.(this.getSnapshot());
}

if (this.status === ActorStatus.Stopped) {
observer.complete?.();
this.observers.delete(observer);
Expand Down
17 changes: 11 additions & 6 deletions packages/core/src/waitFor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,18 @@ export function waitFor<TActorRef extends ActorRef<any, any>>(
sub?.unsubscribe();
};

function checkEmitted(emitted: SnapshotFrom<TActorRef>) {
if (predicate(emitted)) {
dispose();
res(emitted);
}
}

// See if the current snapshot already matches the predicate
checkEmitted(actorRef.getSnapshot());

const sub = actorRef.subscribe({
next: (emitted) => {
if (predicate(emitted)) {
dispose();
res(emitted);
}
},
next: checkEmitted,
error: (err) => {
dispose();
rej(err);
Expand Down
8 changes: 6 additions & 2 deletions packages/core/test/input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,16 @@ describe('input', () => {

const observableActor = interpret(observableBehavior, {
input: { count: 42 }
}).start();
});

observableActor.subscribe((state) => {
const sub = observableActor.subscribe((state) => {
if (state?.count !== 42) return;
expect(state).toEqual({ count: 42 });
done();
sub.unsubscribe();
});

observableActor.start();
});

it('should create a callback actor with input', (done) => {
Expand Down
30 changes: 15 additions & 15 deletions packages/core/test/interpreter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,22 +202,22 @@ describe('interpreter', () => {
}
});

it('should notify subscribers of the current state upon subscription (subscribe)', (done) => {
it('should not notify subscribers of the current state upon subscription (subscribe)', () => {
const spy = jest.fn();
const service = interpret(machine).start();

service.subscribe((state) => {
expect(state.value).toBe('active');
done();
});
service.subscribe(spy);

expect(spy).not.toHaveBeenCalled();
});

it('should notify subscribers of the current state upon subscription (onTransition)', (done) => {
it('should not notify subscribers of the current state upon subscription (onTransition)', () => {
const spy = jest.fn();
const service = interpret(machine).start();

service.onTransition((state) => {
expect(state.value).toBe('active');
done();
});
service.onTransition(spy);

expect(spy).not.toHaveBeenCalled();
});
});

Expand Down Expand Up @@ -1339,20 +1339,20 @@ describe('interpreter', () => {

const sub = toggleService.subscribe(listener);

expect(stateCount).toEqual(1);
expect(stateCount).toEqual(0);

toggleService.send({ type: 'TOGGLE' });

expect(stateCount).toEqual(2);
expect(stateCount).toEqual(1);

toggleService.send({ type: 'TOGGLE' });

expect(stateCount).toEqual(3);
expect(stateCount).toEqual(2);

sub.unsubscribe();
toggleService.send({ type: 'TOGGLE' });

expect(stateCount).toEqual(3);
expect(stateCount).toEqual(2);
});
});

Expand Down Expand Up @@ -1474,7 +1474,7 @@ describe('interpreter', () => {
},
error: undefined,
complete: () => {
expect(count).toEqual(6);
expect(count).toEqual(5);
done();
}
});
Expand Down
5 changes: 0 additions & 5 deletions packages/xstate-inspect/src/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,6 @@ export function inspect(options?: InspectorOptions): Inspector | undefined {
}

service.subscribe((state) => {
// filter out synchronous notification from within `.start()` call
// when the `service.state` has not yet been assigned
if (state === undefined) {
return;
}
inspectService.send({
type: 'service.state',
// TODO: investigate usage of structuredClone in browsers if available
Expand Down
4 changes: 2 additions & 2 deletions packages/xstate-inspect/test/inspect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,12 +123,12 @@ describe('@xstate/inspect', () => {

const service = interpret(machine).start();

expect(() => devTools.register(service)).not.toThrow();

service.send({
type: 'CIRCULAR',
value: circularStructure
});

expect(() => devTools.register(service)).not.toThrow();
});

it('should accept a serializer', () => {
Expand Down
32 changes: 0 additions & 32 deletions packages/xstate-react/test/useInterpret.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,38 +11,6 @@ afterEach(() => {
});

describeEachReactMode('useInterpret (%s)', ({ suiteKey, render }) => {
it('observer should be called with initial state', () => {
let initialState: any;

const machine = createMachine({
initial: 'inactive',
states: {
inactive: {
on: {
ACTIVATE: 'active'
}
},
active: {}
}
});

const App = () => {
const service = useInterpret(machine);

React.useEffect(() => {
service.subscribe((state) => {
initialState ??= state;
});
}, [service]);

return null;
};

render(<App />);

expect(initialState.matches('inactive')).toBeTruthy();
});

it('observer should be called with next state', (done) => {
const machine = createMachine({
initial: 'inactive',
Expand Down
31 changes: 1 addition & 30 deletions packages/xstate-solid/test/createService.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,38 +2,9 @@
import { createMachine } from 'xstate';
import { render, fireEvent, screen } from 'solid-testing-library';
import { createService } from '../src/index.js';
import { createEffect, onMount } from 'solid-js';
import { createEffect } from 'solid-js';

describe('createService', () => {
it('observer should be called with initial state', (done) => {
const machine = createMachine({
initial: 'inactive',
states: {
inactive: {
on: {
ACTIVATE: 'active'
}
},
active: {}
}
});

const App = () => {
const service = createService(machine);

onMount(() => {
service.subscribe((state) => {
expect(state.matches('inactive')).toBeTruthy();
done();
});
});

return null;
};

render(() => <App />);
});

it('observer should be called with next state', (done) => {
const machine = createMachine({
initial: 'inactive',
Expand Down
File renamed without changes.
6 changes: 4 additions & 2 deletions packages/xstate-solid/test/selector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { useActor, createService, useMachine } from '../src/index.js';
import { createMemo, createSignal, from } from 'solid-js';

describe('usage of selectors with reactive service state', () => {
it('only rerenders for selected values', () => {
// TODO: rewrite this test to not use `from()`
it.skip('only rerenders for selected values', () => {
const machine = createMachine<{ count: number; other: number }>({
initial: 'active',
context: {
Expand Down Expand Up @@ -70,7 +71,8 @@ describe('usage of selectors with reactive service state', () => {
expect(countButton.textContent).toBe('2');
});

it('should work with a custom comparison function', () => {
// TODO: rewrite this test to not use `from()`
it.skip('should work with a custom comparison function', () => {
const machine = createMachine<{ name: string }>({
initial: 'active',
context: {
Expand Down
3 changes: 2 additions & 1 deletion packages/xstate-svelte/test/UseSelector.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@
const service = interpret(machine).start();
const state = useSelector(service, (state) => state);
const count = useSelector(service, (state) => state.context.count);
let withSelector = 0;
$: $count && withSelector++;
let withoutSelector = 0;
$: $service.context.count && withoutSelector++;
$: $state.context.count && withoutSelector++;
</script>

<button data-testid="count" on:click={() => service.send({ type: 'INCREMENT' })}
Expand Down
2 changes: 1 addition & 1 deletion packages/xstate-vue/test/UseInterpret.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ const machine = createMachine({
export default defineComponent({
setup() {
const state = ref();
const service = useInterpret(machine, {}, (nextState) => {
state.value = nextState.value;
});
const state = ref(service.getSnapshot().value);
return { service, state };
}
Expand Down
7 changes: 0 additions & 7 deletions packages/xstate-vue/test/useInterpret.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,6 @@ import { render, fireEvent, waitFor } from '@testing-library/vue';
import UseInterpret from './UseInterpret.vue';

describe('useInterpret composable function', () => {
it('observer should be called with initial state', async () => {
const { getByTestId } = render(UseInterpret);

const buttonEl = getByTestId('button');
await waitFor(() => expect(buttonEl.textContent).toBe('Turn on'));
});

it('observer should be called with next state', async () => {
const { getByTestId } = render(UseInterpret);

Expand Down

0 comments on commit 1269470

Please sign in to comment.