Skip to content

fix(js-component-bindgen): treat undefined as none when lowering options#1722

Merged
vados-cosmonic merged 1 commit into
bytecodealliance:mainfrom
GamePad64:fix/option-lower-undefined-none
Jul 4, 2026
Merged

fix(js-component-bindgen): treat undefined as none when lowering options#1722
vados-cosmonic merged 1 commit into
bytecodealliance:mainfrom
GamePad64:fix/option-lower-undefined-none

Conversation

@GamePad64

Copy link
Copy Markdown
Contributor

_lowerFlatOption mapped only JS null to the option none case, but jco's own generated TypeScript types model option<T> none as T | undefined. A value of undefined was therefore lowered as some(undefined); when T contains an own<resource>, _lowerFlatOwn then throws missing resource and aborts the task.

This surfaced with wasi:http p3 response.consume-body, whose trailers future is typed Result<Trailers | undefined, ErrorCode>: an empty-trailers Ok(None) ({ tag: 'ok', val: undefined }) crashed lowering at body completion, leaving the enclosing streaming result's .next() unresolved.

Widen the none check to accept undefined, consistent with the generated types.

@GamePad64 GamePad64 marked this pull request as ready for review July 3, 2026 11:39

@vados-cosmonic vados-cosmonic 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 🚀

Thanks for this fix @GamePad64 !

I really think that we should be leaning towards null here other than undefined, but the current type representation documentation it looks like we really do use undefined.

This is probably something left for a breaking change in Jco for the future -- null is an explicit value (that is none-ish) whereas undefined overlaps with a completely missing option, which makes null seem like the better representation here (there are some benefits like when making objects for record values)... That's a conversation for a different time, I guess.

`_lowerFlatOption` mapped only JS `null` to the option `none` case, but jco's own generated TypeScript types model `option<T>` none as `T | undefined`. A value of `undefined` was therefore lowered as `some(undefined)`; when `T` contains an `own<resource>`, `_lowerFlatOwn` then throws `missing resource` and aborts the task.

This surfaced with wasi:http p3 `response.consume-body`, whose trailers future is typed `Result<Trailers | undefined, ErrorCode>`: an empty-trailers `Ok(None)` (`{ tag: 'ok', val: undefined }`) crashed lowering at body completion, leaving the enclosing streaming result's `.next()` unresolved.

Widen the none check to accept `undefined`, consistent with the generated types.

Signed-off-by: Alexander Shishenko <alex@shishenko.com>
@vados-cosmonic vados-cosmonic force-pushed the fix/option-lower-undefined-none branch from 2c2dd76 to f779305 Compare July 4, 2026 11:07
@vados-cosmonic vados-cosmonic enabled auto-merge July 4, 2026 11:08
@vados-cosmonic vados-cosmonic added this pull request to the merge queue Jul 4, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 4, 2026
@vados-cosmonic vados-cosmonic added this pull request to the merge queue Jul 4, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jul 4, 2026
@vados-cosmonic vados-cosmonic merged commit 4d2f80c into bytecodealliance:main Jul 4, 2026
49 checks passed
GamePad64 added a commit to actcore/host-browser that referenced this pull request Jul 5, 2026
Two stacked bugs made any browser component doing a real wasi:http call inside a streaming tool-result hang indefinitely (ACT-153):

- jco's `_lowerFlatOption` treats only JS `null` as option `none`, but jco's generated types model option-none as `T | undefined`. The empty trailers future (`Ok(None)` = `{tag:'ok', val: undefined}`) was mis-lowered as `some(undefined)`, throwing `missing resource` at body completion and killing the call-tool task. Fixed via a one-line post-transpile patch in patches.ts (upstream: bytecodealliance/jco#1722; drop the patch once it ships).

- `consumeBody` enqueued the body one byte per chunk (~95 B/s); jco batches array-like stream values via appendReadValue, so enqueue the whole Uint8Array in one chunk.

A 43 KB body went from an effective hang (projected ~7.5 min) to ~70 ms, byte-for-byte identical. Updated the consumeBody test to the batched contract.
GamePad64 added a commit to actcore/actcore.dev that referenced this pull request Jul 5, 2026
ACT-153 (jco browser wasi:http hang) is fixed in @actcore/host, so restore the `install` example (`await _pip.install("humanize")`) to the playground picker and refresh the vendored wasi:http shim (public/host/shims) with the batch-enqueue body drain.

The jco `_lowerFlatOption` undefined-as-none crash fix rides in via @actcore/host's patches.ts (upstream: bytecodealliance/jco#1722). Verified end-to-end: a 43 KB body drains in ~70 ms, byte-for-byte identical (was an effective hang).
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