Refactored code and added await to stop race conditions#507
Refactored code and added await to stop race conditions#507
Conversation
droberts-ctrlo
left a comment
droberts-ctrlo
left a comment
There was a problem hiding this comment.
This appears to behave "better" - looks like the setting of the recovering key wasn't always returning as expected (as it wasn't awaited) but I feel it may need checking further in a test environment just to be certain (local testing works fine).
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
|
|
||
| await this.storage.setItem('recovering', true); |
There was a problem hiding this comment.
This was causing a race condition that was borken
| let $list = $("<ul></ul>"); | ||
| const $body = $modal.find(".modal-body"); | ||
| $body.html("<p>Restoring values...</p>").append($list); | ||
| await Promise.all($form.find('.linkspace-field').map(async (_,field) => { |
There was a problem hiding this comment.
Don't need this to be awaited because the finally (after this) encapsulates what would happen at the end either way.
| const $field = $(field); | ||
| await this.storage.getItem(this.columnKey($field)).then(json => { | ||
| try { | ||
| const json = await this.storage.getItem(this.columnKey($field)) |
There was a problem hiding this comment.
I was being a little overzealous with my usage of Promise objects here - IMO this is "cleaner" now; we still need the Promise.all because the field mappings need to run decryption asynchronously.
abeverley
left a comment
abeverley
left a comment
There was a problem hiding this comment.
Thanks Dave, looks good, can you just add some comments inline to the code?
2 participants
No description provided.