Skip to content

Conversation

@sbc100
Copy link
Collaborator

No description provided.

@sbc100
Copy link
CollaboratorAuthor

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
CollaboratorAuthor

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
CollaboratorAuthor

(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.

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

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

@cwoffendencwoffenden 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.

@juj
Copy link
Collaborator

juj commented Dec 9, 2025

I would recommend avoiding this, as it is not necessary, does not provide benefit, and has a code size regression.

Sign up for freeto 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

@sbc100@juj@cwoffenden