Skip to content

Commit

Permalink
Fix persisting unresolved promises (#3959)
Browse files Browse the repository at this point in the history
* Fix persisting unresolved promises

* Changeset

* Avoid mutations of the internal state in all actor types (#3970)

---------

Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
  • Loading branch information
davidkpiano and Andarist committed Apr 19, 2023
1 parent 423c5ab commit ead2872
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 50 deletions.
5 changes: 5 additions & 0 deletions .changeset/large-cooks-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'xstate': patch
---

Unresolved promises will now be properly persisted. The current behavior is to restart a promise that is unresolved.
2 changes: 1 addition & 1 deletion packages/core/src/actors/callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function fromCallback<TEvent extends EventObject>(
if (_event.name === startSignalType) {
const sender = (eventForParent: AnyEventObject) => {
if (state.canceled) {
return state;
return;
}

self._parent?.send(toSCXMLEvent(eventForParent, { origin: self }));
Expand Down
79 changes: 47 additions & 32 deletions packages/core/src/actors/observable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ import { stopSignalType } from '../actors';

export interface ObservableInternalState<T> {
subscription: Subscription | undefined;
canceled: boolean;
status: 'active' | 'done' | 'error';
status: 'active' | 'done' | 'error' | 'canceled';
data: T | undefined;
input?: any;
}
Expand Down Expand Up @@ -43,13 +42,12 @@ export function fromObservable<T, TEvent extends EventObject>(
transition: (state, event, { self, id, defer }) => {
const _event = toSCXMLEvent(event);

if (state.canceled) {
if (state.status !== 'active') {
return state;
}

switch (_event.name) {
case nextEventType:
state.data = event.data.data;
// match the exact timing of events sent by machines
// send actions are not executed immediately
defer(() => {
Expand All @@ -63,29 +61,40 @@ export function fromObservable<T, TEvent extends EventObject>(
)
);
});
return state;
return {
...state,
data: event.data.data
};
case errorEventType:
state.status = 'error';
delete state.input;
state.data = _event.data.data;
return state;
return {
...state,
status: 'error',
input: undefined,
data: _event.data.data,
subscription: undefined
};
case completeEventType:
state.status = 'done';
delete state.input;
return state;
return {
...state,
status: 'done',
input: undefined,
subscription: undefined
};
case stopSignalType:
state.canceled = true;
delete state.input;
state.subscription!.unsubscribe();
return state;
return {
...state,
status: 'canceled',
input: undefined,
subscription: undefined
};
default:
return state;
}
},
getInitialState: (_, input) => {
return {
subscription: undefined,
canceled: false,
status: 'active',
data: undefined,
input
Expand All @@ -109,8 +118,7 @@ export function fromObservable<T, TEvent extends EventObject>(
});
},
getSnapshot: (state) => state.data,
getPersistedState: ({ canceled, status, data, input }) => ({
canceled,
getPersistedState: ({ status, data, input }) => ({
status,
data,
input
Expand Down Expand Up @@ -150,33 +158,41 @@ export function fromEventObservable<T extends EventObject>(
transition: (state, event) => {
const _event = toSCXMLEvent(event);

if (state.canceled) {
if (state.status !== 'active') {
return state;
}

switch (_event.name) {
case errorEventType:
state.status = 'error';
delete state.input;
state.data = _event.data.data;
return state;
return {
...state,
status: 'error',
input: undefined,
data: _event.data.data,
subscription: undefined
};
case completeEventType:
state.status = 'done';
delete state.input;
return state;
return {
...state,
status: 'done',
input: undefined,
subscription: undefined
};
case stopSignalType:
state.canceled = true;
delete state.input;
state.subscription!.unsubscribe();
return state;
return {
...state,
status: 'canceled',
input: undefined,
subscription: undefined
};
default:
return state;
}
},
getInitialState: () => {
return {
subscription: undefined,
canceled: false,
status: 'active',
data: undefined
};
Expand All @@ -200,8 +216,7 @@ export function fromEventObservable<T extends EventObject>(
});
},
getSnapshot: (_) => undefined,
getPersistedState: ({ canceled, status, data, input }) => ({
canceled,
getPersistedState: ({ status, data, input }) => ({
status,
data,
input
Expand Down
34 changes: 19 additions & 15 deletions packages/core/src/actors/promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ import { toSCXMLEvent } from '../utils';
import { stopSignalType } from '../actors';

export interface PromiseInternalState<T> {
status: 'active' | 'error' | 'done';
canceled: boolean;
status: 'active' | 'error' | 'done' | 'canceled';
data: T | undefined;
input?: any;
}
Expand Down Expand Up @@ -33,27 +32,33 @@ export function fromPromise<T>(
transition: (state, event) => {
const _event = toSCXMLEvent(event);

if (state.canceled) {
if (state.status !== 'active') {
return state;
}

const eventObject = _event.data;

switch (_event.name) {
case resolveEventType:
state.status = 'done';
state.data = eventObject.data;
delete state.input;
return state;
return {
...state,
status: 'done',
data: eventObject.data,
input: undefined
};
case rejectEventType:
state.status = 'error';
state.data = eventObject.data;
delete state.input;
return state;
return {
...state,
status: 'error',
data: eventObject.data,
input: undefined
};
case stopSignalType:
state.canceled = true;
delete state.input;
return state;
return {
...state,
status: 'canceled',
input: undefined
};
default:
return state;
}
Expand All @@ -80,7 +85,6 @@ export function fromPromise<T>(
},
getInitialState: (_, input) => {
return {
canceled: false,
status: 'active',
data: undefined,
input
Expand Down
28 changes: 26 additions & 2 deletions packages/core/test/behaviors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,31 @@ describe('promise behavior (fromPromise)', () => {
expect(called).toBe(false);
});

it('should persist a promise', (done) => {
it('should persist an unresolved promise', (done) => {
const promiseBehavior = fromPromise(
() =>
new Promise<number>((res) => {
setTimeout(() => res(42), 10);
})
);

const actor = interpret(promiseBehavior);
actor.start();

const resolvedPersistedState = actor.getPersistedState();
actor.stop();

const restoredActor = interpret(promiseBehavior, {
state: resolvedPersistedState
}).start();

setTimeout(() => {
expect(restoredActor.getSnapshot()).toBe(42);
done();
}, 20);
});

it('should persist a resolved promise', (done) => {
const promiseBehavior = fromPromise(
() =>
new Promise<number>((res) => {
Expand Down Expand Up @@ -350,7 +374,7 @@ describe('machine behavior', () => {

expect(persistedState.children.a.state).toEqual(
expect.objectContaining({
canceled: false,
status: 'done',
data: 42
})
);
Expand Down

0 comments on commit ead2872

Please sign in to comment.