Skip to content

Conversation

mishig25
Copy link
Collaborator

@mishig25 mishig25 commented Apr 3, 2023

await is probably missing on that line

@mishig25 mishig25 requested review from coyotte508 and radames April 3, 2023 07:58
Copy link
Member

@coyotte508 coyotte508 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice it's not needed, when returning promises, await doesn't change the behavior (except maybe a little more overhead / debug callstack)

Feel free to merge!

@mishig25 mishig25 merged commit 99f9eed into main Apr 3, 2023
@mishig25 mishig25 deleted the fix_fillMask branch April 3, 2023 12:55
@Aschen
Copy link
Contributor

Aschen commented Apr 3, 2023

Just my 2 cents here, adding await will create 2 additional promises on the event loop (that will resolve on next ticks)

It only made sens to have return await when you want to catch early:

try {
  return await ...;
} catch (error) {
  
}

@coyotte508
Copy link
Member

Thanks for the info!

In any case, #125 overwrites this part of the code so this was a temporary change x)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants