Skip to content

fix(crypto): remove setImmediate from async calls#1637

Merged
AngelCastilloB merged 1 commit into
masterfrom
fix/remove-setImmediate
Jun 25, 2025
Merged

fix(crypto): remove setImmediate from async calls#1637
AngelCastilloB merged 1 commit into
masterfrom
fix/remove-setImmediate

Conversation

@AngelCastilloB

Copy link
Copy Markdown
Member

Context

setImmediate is not available in browser environments, and we dont really need it.

Proposed Solution

Remove calls to setImmediate

@iccicci iccicci left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IDK if blake2b.hash<T>(message, outputLengthBytes) returns a Promise, I guess yes, otherwise we would have had a type error.
If that's the case, wouldn't be better to remove the async attribute from the function definition?

@AngelCastilloB

Copy link
Copy Markdown
Member Author

IDK if blake2b.hash<T>(message, outputLengthBytes) returns a Promise, I guess yes, otherwise we would have had a type error. If that's the case, wouldn't be better to remove the async attribute from the function definition?

blake2b.hash doesnt return a promise, but because is inside an async function it will be wrapped inside a promise that resolves immediately. We need the function to be async to match the interface, the mobile implementataion uses an async provider for this interface

@iccicci iccicci left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@mirceahasegan mirceahasegan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm

@AngelCastilloB AngelCastilloB merged commit 22204c7 into master Jun 25, 2025
10 of 11 checks passed
@AngelCastilloB AngelCastilloB deleted the fix/remove-setImmediate branch June 25, 2025 09:41
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