Skip to content

Commit

Permalink
Fix (Core): Fix missed re-renders from useEffect (Fix #1282)
Browse files Browse the repository at this point in the history
  • Loading branch information
SBoudrias committed Aug 17, 2023
1 parent 319d2d9 commit 0d9dac4
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 23 deletions.
7 changes: 6 additions & 1 deletion packages/core/core.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,9 @@ describe('createPrompt()', () => {

useEffect(() => {
setValue('foo');
}, []);

useEffect(() => {
setKey('bar');
}, []);

Expand All @@ -286,8 +289,10 @@ describe('createPrompt()', () => {
};

const prompt = createPrompt(Prompt);
const { answer, events } = await render(prompt, { message: 'Question' });
const { answer, events, getScreen } = await render(prompt, { message: 'Question' });

expect(renderSpy).toHaveBeenCalledTimes(2);
expect(getScreen()).toMatchInlineSnapshot('"Question bar:foo"');

events.keypress('enter');
await expect(answer).resolves.toEqual('foo');
Expand Down
45 changes: 23 additions & 22 deletions packages/core/src/index.mts
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,6 @@ function resetHookState() {
sessionRl = undefined;
}

function cleanupHook(index: number) {
const cleanFn = hooksCleanup[index];
if (typeof cleanFn === 'function') {
cleanFn();
}
}

function mergeStateUpdates<T extends (...args: any) => any>(
fn: T,
): (...args: Parameters<T>) => ReturnType<T> {
Expand All @@ -70,6 +63,19 @@ function mergeStateUpdates<T extends (...args: any) => any>(
return wrapped;
}

function cleanupHook(index: number) {
const cleanFn = hooksCleanup[index];
if (typeof cleanFn === 'function') {
cleanFn();
}
}

const runEffects = mergeStateUpdates(() => {
for (const effect of hooksEffect) {
effect();
}
});

export function useState<Value>(
defaultValue: NotFunction<Value> | (() => Value),
): [Value, (newValue: Value) => void] {
Expand Down Expand Up @@ -117,18 +123,14 @@ export function useEffect(
hasChanged = depArray.some((dep, i) => !Object.is(dep, oldDeps[i]));
}
if (hasChanged) {
hooksEffect.push(
mergeStateUpdates(() => {
cleanupHook(_idx);
const cleanFn = cb(rl);
if (cleanFn != null && typeof cleanFn !== 'function') {
throw new Error(
'useEffect return value must be a cleanup function or nothing.',
);
}
hooksCleanup[_idx] = cleanFn;
}),
);
hooksEffect.push(() => {
cleanupHook(_idx);
const cleanFn = cb(rl);
if (cleanFn != null && typeof cleanFn !== 'function') {
throw new Error('useEffect return value must be a cleanup function or nothing.');
}
hooksCleanup[_idx] = cleanFn;
});
}
hooks[_idx] = depArray;
}
Expand Down Expand Up @@ -304,13 +306,12 @@ export function createPrompt<Value, Config extends AsyncPromptConfig>(

try {
const nextView = view(resolvedConfig, done);
for (const effect of hooksEffect) {
effect();
}

const [content, bottomContent] =
typeof nextView === 'string' ? [nextView] : nextView;
screen.render(content, bottomContent);

runEffects();
} catch (err) {
onExit();
reject(err);
Expand Down

0 comments on commit 0d9dac4

Please sign in to comment.