mirror of
https://github.com/duckietm/Nitro-V3.git
synced 2026-06-19 15:06:20 +00:00
fix(useWordQuizWidget): closure-captured stale userAnswers + useRef for timeout handle
Two bugs and one tidy in the word-quiz widget hook.
== Bug 1: stale-closure read in setUserAnswers updater
setUserAnswers(prevValue => {
if(!prevValue.has(userData.roomIndex)) {
const newValue = new Map(userAnswers); // <- WRONG: reads
// ^^^^^^^ the closed-over
// state, not prevValue
newValue.set(userData.roomIndex, ...);
return newValue;
}
return prevValue;
});
The functional updater is supposed to read the *latest* state (its
`prevValue` argument), not the closed-over `userAnswers` from the
render that registered this listener. The old code mixed both:
`prevValue.has(...)` for the check but `new Map(userAnswers)` for the
copy. Under rapid successive ANSWERED events for different users
within the same tick, the second update would copy a stale map and
drop the first user's entry. Fixed: use prevValue throughout.
== Bug 2: questionClearTimeout stored in useState
The timeout handle is a side-channel value, not display state. Storing
it in useState meant every (re)schedule triggered a re-render even
though no widget reads it. It also let the cleanup effect close over
a stale handle if the unmount fired between the schedule and the
state commit. Moved to useRef + a small `scheduleQuestionClear(delay)`
helper that consolidates the clear-then-set pattern (was duplicated
across FINISHED and QUESTION handlers).
== Tidy
- The duration-zero branch of QUESTION now explicitly clears any
pending timeout instead of falling through to a `setTimeout(..., null)`
no-op path.
- Cleanup effect rewritten as a single arrow-return for brevity.
Public API of useWordQuizWidget unchanged. Suite: 207/207.
This commit is contained in:
@@ -1,5 +1,5 @@
|
|||||||
import { AvatarAction, GetRoomEngine, IQuestion, RoomSessionWordQuizEvent } from '@nitrots/nitro-renderer';
|
import { AvatarAction, GetRoomEngine, IQuestion, RoomSessionWordQuizEvent } from '@nitrots/nitro-renderer';
|
||||||
import { useEffect, useState } from 'react';
|
import { useEffect, useRef, useState } from 'react';
|
||||||
import { VoteValue } from '../../../api';
|
import { VoteValue } from '../../../api';
|
||||||
import { useNitroEvent } from '../../events';
|
import { useNitroEvent } from '../../events';
|
||||||
import { useRoom } from '../useRoom';
|
import { useRoom } from '../useRoom';
|
||||||
@@ -13,9 +13,13 @@ const useWordQuizWidgetState = () =>
|
|||||||
const [ pollId, setPollId ] = useState(-1);
|
const [ pollId, setPollId ] = useState(-1);
|
||||||
const [ question, setQuestion ] = useState<IQuestion>(null);
|
const [ question, setQuestion ] = useState<IQuestion>(null);
|
||||||
const [ answerSent, setAnswerSent ] = useState(false);
|
const [ answerSent, setAnswerSent ] = useState(false);
|
||||||
const [ questionClearTimeout, setQuestionClearTimeout ] = useState<ReturnType<typeof setTimeout>>(null);
|
|
||||||
const [ answerCounts, setAnswerCounts ] = useState<Map<string, number>>(new Map());
|
const [ answerCounts, setAnswerCounts ] = useState<Map<string, number>>(new Map());
|
||||||
const [ userAnswers, setUserAnswers ] = useState<Map<number, VoteValue>>(new Map());
|
const [ userAnswers, setUserAnswers ] = useState<Map<number, VoteValue>>(new Map());
|
||||||
|
// The question-clear timeout is a side-channel handle, not display
|
||||||
|
// state — storing it in a ref avoids a re-render every time we
|
||||||
|
// (re)schedule it and lets the cleanup effect read the *latest*
|
||||||
|
// handle on unmount instead of the closed-over one.
|
||||||
|
const questionClearTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
|
||||||
const { answerPoll } = usePollActions();
|
const { answerPoll } = usePollActions();
|
||||||
const { roomSession = null } = useRoom();
|
const { roomSession = null } = useRoom();
|
||||||
|
|
||||||
@@ -25,6 +29,17 @@ const useWordQuizWidgetState = () =>
|
|||||||
setQuestion(null);
|
setQuestion(null);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
const scheduleQuestionClear = (delay: number) =>
|
||||||
|
{
|
||||||
|
if(questionClearTimeoutRef.current) clearTimeout(questionClearTimeoutRef.current);
|
||||||
|
|
||||||
|
questionClearTimeoutRef.current = setTimeout(() =>
|
||||||
|
{
|
||||||
|
questionClearTimeoutRef.current = null;
|
||||||
|
clearQuestion();
|
||||||
|
}, delay);
|
||||||
|
};
|
||||||
|
|
||||||
const vote = (vote: string) =>
|
const vote = (vote: string) =>
|
||||||
{
|
{
|
||||||
if(answerSent || !question) return;
|
if(answerSent || !question) return;
|
||||||
@@ -44,16 +59,15 @@ const useWordQuizWidgetState = () =>
|
|||||||
|
|
||||||
setUserAnswers(prevValue =>
|
setUserAnswers(prevValue =>
|
||||||
{
|
{
|
||||||
if(!prevValue.has(userData.roomIndex))
|
// Bug fix: previously this read the closure-captured `userAnswers`
|
||||||
{
|
// (which was stale by one render) instead of `prevValue`, so
|
||||||
const newValue = new Map(userAnswers);
|
// rapid successive ANSWERED events for different users could
|
||||||
|
// overwrite each other. Use prevValue.
|
||||||
|
if(prevValue.has(userData.roomIndex)) return prevValue;
|
||||||
|
|
||||||
newValue.set(userData.roomIndex, { value: event.value, secondsLeft: SIGN_FADE_DELAY });
|
const next = new Map(prevValue);
|
||||||
|
next.set(userData.roomIndex, { value: event.value, secondsLeft: SIGN_FADE_DELAY });
|
||||||
return newValue;
|
return next;
|
||||||
}
|
|
||||||
|
|
||||||
return prevValue;
|
|
||||||
});
|
});
|
||||||
|
|
||||||
GetRoomEngine().updateRoomObjectUserGesture(roomSession.roomId, userData.roomIndex, AvatarAction.getGestureId((event.value === '0') ? AvatarAction.GESTURE_SAD : AvatarAction.GESTURE_SMILE));
|
GetRoomEngine().updateRoomObjectUserGesture(roomSession.roomId, userData.roomIndex, AvatarAction.getGestureId((event.value === '0') ? AvatarAction.GESTURE_SAD : AvatarAction.GESTURE_SMILE));
|
||||||
@@ -66,12 +80,7 @@ const useWordQuizWidgetState = () =>
|
|||||||
setAnswerCounts(event.answerCounts);
|
setAnswerCounts(event.answerCounts);
|
||||||
setAnswerSent(true);
|
setAnswerSent(true);
|
||||||
|
|
||||||
setQuestionClearTimeout(prevValue =>
|
scheduleQuestionClear(DEFAULT_DISPLAY_DELAY);
|
||||||
{
|
|
||||||
if(prevValue) clearTimeout(prevValue);
|
|
||||||
|
|
||||||
return setTimeout(() => clearQuestion(), DEFAULT_DISPLAY_DELAY);
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|
||||||
setUserAnswers(new Map());
|
setUserAnswers(new Map());
|
||||||
@@ -85,24 +94,21 @@ const useWordQuizWidgetState = () =>
|
|||||||
setAnswerCounts(new Map());
|
setAnswerCounts(new Map());
|
||||||
setUserAnswers(new Map());
|
setUserAnswers(new Map());
|
||||||
|
|
||||||
setQuestionClearTimeout(prevValue =>
|
if(event.duration > 0)
|
||||||
{
|
{
|
||||||
if(prevValue) clearTimeout(prevValue);
|
const delay = event.duration < 1000 ? DEFAULT_DISPLAY_DELAY : event.duration;
|
||||||
|
scheduleQuestionClear(delay);
|
||||||
if(event.duration > 0)
|
}
|
||||||
{
|
else if(questionClearTimeoutRef.current)
|
||||||
const delay = event.duration < 1000 ? DEFAULT_DISPLAY_DELAY : event.duration;
|
{
|
||||||
|
clearTimeout(questionClearTimeoutRef.current);
|
||||||
return setTimeout(() => clearQuestion(), delay);
|
questionClearTimeoutRef.current = null;
|
||||||
}
|
}
|
||||||
|
|
||||||
return null;
|
|
||||||
});
|
|
||||||
});
|
});
|
||||||
|
|
||||||
useEffect(() =>
|
useEffect(() =>
|
||||||
{
|
{
|
||||||
const checkSignFade = () =>
|
const tick = () =>
|
||||||
{
|
{
|
||||||
setUserAnswers(prevValue =>
|
setUserAnswers(prevValue =>
|
||||||
{
|
{
|
||||||
@@ -117,30 +123,24 @@ const useWordQuizWidgetState = () =>
|
|||||||
|
|
||||||
if(keysToRemove.length === 0) return prevValue;
|
if(keysToRemove.length === 0) return prevValue;
|
||||||
|
|
||||||
const copy = new Map(prevValue);
|
const next = new Map(prevValue);
|
||||||
|
keysToRemove.forEach(key => next.delete(key));
|
||||||
keysToRemove.forEach(key => copy.delete(key));
|
return next;
|
||||||
|
|
||||||
return copy;
|
|
||||||
});
|
});
|
||||||
};
|
};
|
||||||
|
|
||||||
const interval = setInterval(() => checkSignFade(), 1000);
|
const interval = setInterval(tick, 1000);
|
||||||
|
|
||||||
return () => clearInterval(interval);
|
return () => clearInterval(interval);
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
useEffect(() =>
|
useEffect(() => () =>
|
||||||
{
|
{
|
||||||
return () =>
|
if(questionClearTimeoutRef.current)
|
||||||
{
|
{
|
||||||
setQuestionClearTimeout(prevValue =>
|
clearTimeout(questionClearTimeoutRef.current);
|
||||||
{
|
questionClearTimeoutRef.current = null;
|
||||||
if(prevValue) clearTimeout(prevValue);
|
}
|
||||||
|
|
||||||
return null;
|
|
||||||
});
|
|
||||||
};
|
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
return { question, answerSent, answerCounts, userAnswers, vote };
|
return { question, answerSent, answerCounts, userAnswers, vote };
|
||||||
|
|||||||
Reference in New Issue
Block a user