Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Dec 3, 2025

No description provided.

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 3, 2025

View with "hide whitespace"

@juj
Copy link
Collaborator

juj commented Dec 4, 2025

What does using 'async' provide here? It increases code size, but otherwise seems nonfunctional?

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 4, 2025

Just modernizing the code, but maybe not worth in this case? If I can't find a way to make it is at least size neutral maybe I will just abandon.

@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 4, 2025

(I do think it makes the code more readable in the modern form, avoid extra indentation etc, but not worth it in this case probably)

@cwoffenden
Copy link
Collaborator

My understanding is, async doesn't gain us anything here because these functions are never passed to promises, and only used from wasm. I'm sure it's the correct, idiomatic way of writing JS though.

@sbc100 sbc100 marked this pull request as draft December 8, 2025 17:34
@sbc100
Copy link
Collaborator Author

sbc100 commented Dec 8, 2025

My understanding is, async doesn't gain us anything here because these functions are never passed to promises, and only used from wasm. I'm sure it's the correct, idiomatic way of writing JS though.

Correct this would just be an NFC, stylistic change.

Copy link
Collaborator

@cwoffenden cwoffenden left a comment

Choose a reason for hiding this comment

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

LGTM. The gzipped JS and wasm size stay the same.

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