Skip to content

BridgeJS: Unify @JS struct parameter lowering onto the stack ABI - #798

Merged
krodak merged 1 commit into
mainfrom
yt/fix-struct-closure-param
Aug 10, 2026
Merged

BridgeJS: Unify @JS struct parameter lowering onto the stack ABI#798
krodak merged 1 commit into
mainfrom
yt/fix-struct-closure-param

Conversation

@kateinoigakukun

@kateinoigakukunkateinoigakukun commented Aug 10, 2026

Copy link
Copy Markdown
Member

Summary

@JS structbridgeJSLowerParameter() used toJSObject() (object-id ABI). Swift→JS callbacks and the rest of the stack ABI expect a stack push + JS structHelpers.*.lift(). The object-id path pushed then lifted into a JS object, discarded the id, and left the stacks empty for the callback glue — so JS read garbage.

This matches arrays/dictionaries: lower = stack push. ImportTS non-optional structs use the same stack ABI (optional structs already did).

Problematic case

@JSstructVector2D{vardx:Doublevardy:Double}@JSclassTextProcessor{@JSfunc observeVector(_ callback:(Vector2D)->Void){callback(Vector2D(dx:1.5, dy:2.5))}}
processor.observeVector((vector)=>{// Before: { dx: undefined, dy: undefined, ... }// After: { dx: 1.5, dy: 2.5, ... }});

Same failure mode for any Swift→JS typed closure that takes a non-optional @JS struct (e.g. observation payloads). Bool fields were especially misleading: empty-stack i32 pops yielded undefined, and undefined !== 0 became true.

Fix

  • _BridgedSwiftStruct.bridgeJSLowerParameter()bridgeJSStackPush()
  • ImportTS non-optional struct param/return ABI → stack (same as ExportSwift / optionals)
  • Runtime regression: observeVector / mapVector in ClosureSupportTests

Test plan

  • BridgeJS plugin tests / snapshots
  • ClosureSupportTests.testRunJsClosureSupportTests (struct callback params)
  • SwiftStructTests.testSwiftStructInImportedSignature (ImportTS stack unify)

bridgeJSLowerParameter for structs used toJSObject(), so Swift→JS
callbacks discarded the stack and JS lift() read garbage. Match arrays:
stack-push on lower, and use the same ABI for ImportTS non-optional structs.
@kateinoigakukun
kateinoigakukun marked this pull request as ready for review August 10, 2026 00:58
@krodak
krodak merged commit 83eae16 into mainAug 10, 2026
31 of 32 checks passed
@krodak
krodak deleted the yt/fix-struct-closure-param branch August 10, 2026 11:48
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.

2 participants

@kateinoigakukun@krodak