This addresses ENG-4285. Will explain changes inline.
Details
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
lib/reducers/message-reducer.js | ||
---|---|---|
1298 ↗ | (On Diff #34323) | We can't simply remove these any-casts because Flow is right that there is an error here. We cannot treat { ...singleMedia, ...mediaUpdate } as a Media, even after refining singleMedia to EncryptedImage and mediaUpdate to Partial<EncryptedImage>. The reason is that EncryptedImage is a union type that sometimes has holder and sometimes has blobURI. If singleMedia has one and mediaUpdate has the other, we end up with both. But having both holder and blobURI is not a valid EncryptedImage. The same issue exists for EncryptedVideo. Ironically, the best solution here is to introduce branching similar to the branching I was able to remove in the later part of the file. |
1299 ↗ | (On Diff #34323) | As an alternative to throwing away holder, we could add an invariant that mediaUpdate does not have one. I wasn't sure what was better |
1383 ↗ | (On Diff #34323) | This was actually easy to remove!! Great to see that the new Flow is not as annoying about unnecessary branching. |
lib/reducers/message-reducer.js | ||
---|---|---|
1298 ↗ | (On Diff #34323) |
In practice such situation should never happen - there's either holder in both or blobURI in both. But looking at types only, that makes sense |
1299 ↗ | (On Diff #34323) | Not sure either, failing with invariant seems like an overkill, however notifying the developer that this happened would be nice to have. |
lib/reducers/message-reducer.js | ||
---|---|---|
1299 ↗ | (On Diff #34323) |
I added a console.log |