Skip to content

[libpromise.js] Add explicit addPromise helper function#26950

Merged
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:addPromise
May 14, 2026
Merged

[libpromise.js] Add explicit addPromise helper function#26950
sbc100 merged 1 commit into
emscripten-core:mainfrom
sbc100:addPromise

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented May 14, 2026

In addition to making things more explicitly, this saves on codesize when more than one promise function is included (see dylink_all codesize changes).

In addition to making things more explicitly, this saves on codesize
when more than one promise function is included (see dylink_all codesize
changes).
@sbc100 sbc100 requested a review from tlively May 14, 2026 16:29
@tlively
Copy link
Copy Markdown
Member

tlively commented May 14, 2026

"explicitly" => "explicit" in the description, but lgtm.

How do you decide when adding a tiny helper like this is worth it for code size benefits? It seems like you would structure lots of code differently if you were really trying to code golf it.

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 14, 2026

"explicitly" => "explicit" in the description, but lgtm.

How do you decide when adding a tiny helper like this is worth it for code size benefits? It seems like you would structure lots of code differently if you were really trying to code golf it.

TBH, what inspired me to make this change was readability. I was working on #26947 and I didn't at first see how promises were added to the mapping other than makePromise .. which creates and extra promise, which I didn't want to do.

Having both makePromise and addPromise side by side makes it clear there are two ways to do things rather than distributing the second method around the codebase.

@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented May 14, 2026

Also this just seemed better than repeating the .. {promise: ...} pattern everywhere

@sbc100 sbc100 merged commit 378cc21 into emscripten-core:main May 14, 2026
30 checks passed
@sbc100 sbc100 deleted the addPromise branch May 14, 2026 20:23
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.

2 participants