This addresses ENG-9187, as well as fixes a more general issue with useProcessAndSendDMOperation:
If the callback from useProcessAndSendDMOperation was called twice in a continuous block of code, the second call would not see the effects of the first call. This is because underneath, dispatchActionPromise is called that changes the state. So for the second call to see the effects of the first call, a render has to happen between the calls.
Details
Details
Tested that when creatiing a new thick thread, the message appears to be in progress and then sent / failed.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Branch
- inka/thread_ops
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/shared/dm-ops/process-dm-ops.js | ||
|---|---|---|
| 315–319 | It would be better to use an object instead of a tuple so that the props are less confusing. Also, we shouldn't use any here - is there a way to avoid this? It seems like we're always calling the resolve without argument and never call the reject. Is it correct that we aren't rejecting at all? | |
| 323 | What is the benefit of introducing this function? Wouldn't it be simpler if we checked the queue length and early exited here? | |
| lib/shared/dm-ops/process-dm-ops.js | ||
|---|---|---|
| 315–318 ↗ | (On Diff #44042) | We should prefer Array type, for mutable arrays. But this one should be immutable. |
Comment Actions
Without this diff, problem from ENG-9187 is not resolved. However
- This diff introduces a different problem - if the component which calls this hook is unmounted before the effect is run, the operation is lost
- We are changing the approach to generating DM ops (D13290), and @kamil will be landing code rendering this diff unnecessary - most likely tomorrow
So we will land the rest of the stack. This doesn't break anything, because for now DM thread creation is hidden behind a flag anyway.