Skip to content

Commit dabf691

Browse files
author
srand
committed
DEBUG ME CHALLENGE
Note: It seems `done` on line 82 is executed once (log on line 88), but the callback of `setMsgs` is executed multiple times (log on line 92). It is expected to execute only once, or line 56 would fail because `custom` is already parsed. More strangely, uncommenting `try-catch` on line 54,57-59 seems to change this multi-execute behavior/bug mysteriously. `catch` seems never executed (log on line 58) and `setMsgs` callback seems only executes once (log on line 92). Theorically it should be related to inplace mutation of `obj.msgs`/`msg.custom`, but the behavior above looks more like a React codegen issue. Tracing up to the multi-execute of callback is hard. However, with `console.trace` or `obj.debug` (a mutating counter), or with `debugger` or breakpoint, we can see that it actually execute 3 times without `try-catch` and 2 times with it, (rather than 2 vs 1). For some reason `console.log` does not work on the 2nd execution (1st and 3rd ok) but `console.trace` works all the time. Similarly, the `catch` on line 58 is executed but its `console.log` is muted because it is during 2nd run. This makes a little bit more sense. With `try-catch`, the 2nd execution is no-op and nothing gets mutated. So it works as expected. Without it, the 2nd execution throws and somehow React decides to run it 3rd time, which finally make React unhappy. It is left unknown why `console.log` is less useful than `console.trace`. And whether/how React's multi-execution of callback is related to mutation of object captured by the closure. But anyways, the action should be a descriptive object and reducer should be pure (or at least idempotent here). React.StrictMode calls the callback twice intentionally. facebook/react#12856 (comment) Note that parsing and reversing was outside `setMsgs` in `HEAD^`, so it executes only once as expected. For performance, we may extract sanitization unrelated to `msgsPrev` (eg. !resend and parsing) outside of `setMsgs`. `console.log` was overwritten as `function disabledLog() {}` by React during the 2nd call to hide its side effects.
1 parent 9ab3dcd commit dabf691

File tree

1 file changed

+43
-4
lines changed

1 file changed

+43
-4
lines changed

src/Room.js

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,41 @@ export const Room = props => {
4141
const ondisconnect = (...args) => { console.log('d', args); };
4242
const onerror = (...args) => { console.log('e', args); };
4343
const onmsgs = msgs => {
44-
msgs.forEach(msg => {msg.custom = JSON.parse(msg.custom || null);});
45-
setMsgs(msgsPrev => [...msgsPrev, ...msgs]);
44+
setMsgs(msgsPrev => {
45+
msgs = sanitizeDelta(msgs, msgsPrev, 'dupNew');
46+
return [...msgsPrev, ...msgs];
47+
});
4648
console.log(msgs);
4749
};
50+
const sanitizeDelta = (msgs, msgsPrev, tag) => {
51+
// debugger;
52+
msgs = msgs.filter(msg => !msg.resend);
53+
msgs.forEach((msg, idx) => {
54+
// try {
55+
// if (msg.custom && typeof msg.custom !== 'string') debugger;
56+
msg.custom = JSON.parse(msg.custom || null);
57+
// } catch (e) {
58+
// console.log('catch', e, msg);
59+
// }
60+
});
61+
// Mostly unnecessary after `resend` check.
62+
// But consecutive button clicks can send dup requests.
63+
const byKey = new Map(msgsPrev.map((msg, idx) => [msg.idClient, idx]));
64+
// console.log('sanitize', byKey, msgs);
65+
const lookup = msgs.map((msg, idx) => [idx, msg, byKey.get(msg.idClient)]);
66+
const dups = lookup.filter(
67+
([idx, msg, idxEx]) => idxEx !== undefined
68+
).map(
69+
([idx, msg, idxEx]) => [idx, msg, idxEx, msgsPrev[idxEx]]
70+
);
71+
if (dups.length > 0) console.log(tag, dups);
72+
msgs = lookup.filter(
73+
([idx, msg, idxEx]) => idxEx === undefined
74+
).map(
75+
([idx, msg, idxEx]) => msg
76+
);
77+
return msgs;
78+
};
4879
const earlier = () => {
4980
chatroom.getHistoryMsgs({
5081
timetag: msgsView[0]?.time,
@@ -54,8 +85,16 @@ export const Room = props => {
5485
return;
5586
}
5687
obj.msgs.reverse();
57-
obj.msgs.forEach(msg => {msg.custom = JSON.parse(msg.custom || null);});
58-
setMsgs(msgsPrev => [...obj.msgs, ...msgsPrev]);
88+
console.log('debug-done');
89+
const doneTime = new Date();
90+
setMsgs(msgsPrev => {
91+
obj.debug = (obj.debug || 0) + 1;
92+
// console.log('debug-setMsgs', doneTime, obj, msgsView);
93+
console.trace('debug-setMsgs', doneTime, obj, msgsView, console.log);
94+
// debugger;
95+
obj.msgs = sanitizeDelta(obj.msgs, msgsPrev, 'dupHist');
96+
return [...obj.msgs, ...msgsPrev];
97+
});
5998
},
6099
});
61100
};

0 commit comments

Comments
 (0)