Skip to content

Commit

Permalink
react-experiment: fix tasks being skipped when onTaskCompleted is cal…
Browse files Browse the repository at this point in the history
…led multiple times (#209)

* react-experiment: prevent completing a task twice

* react-experiment: multiple onTaskCompleted test

* changest
  • Loading branch information
QuentinRoy authored Dec 15, 2023
1 parent 9a37443 commit f6a7a82
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/large-ties-explain.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lightmill/react-experiment': patch
---

Fix tasks being skipped when onTaskCompleted is called several times.
47 changes: 47 additions & 0 deletions packages/react-experiment/__tests__/run.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,51 @@ describe('run', () => {
);
}).toThrow('Could not find task to resume after');
});

it('throws an error if the same task is completed multiple times', async () => {
const wrapper = vi.fn(
(f: () => void, { shouldFail }: { shouldFail: boolean }) => {
if (shouldFail) {
try {
expect(() => f()).toThrowErrorMatchingInlineSnapshot(
`[Error: Task already completed]`,
);
} catch (e) {}
} else {
f();
}
},
);
const BadTask = () => {
let { onTaskCompleted } = useTask('bad');
return (
<div>
<h1>Bad Task</h1>
<button
onClick={() => {
wrapper(onTaskCompleted, { shouldFail: false });
wrapper(onTaskCompleted, { shouldFail: true });
}}
>
Complete
</button>
</div>
);
};
let config = {
tasks: {
bad: <BadTask />,
ok: <Task type="ok" dataProp="prop" />,
},
};
render(
<Run
elements={config}
timeline={[{ type: 'bad' }, { type: 'ok', prop: 'hello' }]}
/>,
);
expect(screen.getByRole('heading')).toHaveTextContent('Bad Task');
fireEvent.click(screen.getByRole('button'));
expect(wrapper).toHaveBeenCalledTimes(2);
});
});
3 changes: 3 additions & 0 deletions packages/react-experiment/src/useManagedTimeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,14 @@ export default function useManagedTimeline<Task extends { type: string }>(
runner.completeTask();
return;
}
let hasBeenCompleted = false;
setState({
status: 'running',
task,
onTaskCompleted() {
if (hasBeenCompleted) throw new Error('Task already completed');
runner.completeTask();
hasBeenCompleted = true;
},
});
optionsRef.current.onTaskStarted?.(task);
Expand Down

0 comments on commit f6a7a82

Please sign in to comment.