Skip to content

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

Open
GamePad64 wants to merge 1 commit into
bytecodealliance:mainfrom
GamePad64:fix/option-lower-undefined-none
Open

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

Conversation

@GamePad64

Copy link
Copy Markdown

_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
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