Skip to content

Add import-side array handling and JSClass array tests #572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
kateinoigakukun wants to merge 7 commits into
base: main
Choose a base branch
from
Draft

Add import-side array handling and JSClass array tests #572

kateinoigakukun wants to merge 7 commits into from

Conversation

@kateinoigakukun
Copy link
Member

@kateinoigakukun kateinoigakukun commented Feb 3, 2026

Motivation:

  • Import-side arrays lacked ordering guarantees when multiple stack-based parameters were present and we were missing coverage for arrays in imported JS class members.

Overview:

  • Reverse stack-based parameter lowering in ImportTS, deduplicate raw-value enum handling, and ensure lift fragments without parameters run so array returns are emitted.
  • Add ArrayHost fixtures and runtime checks for JSClass array properties/methods, wire them into the JS prelude, and regenerate BridgeJS outputs.

@kateinoigakukun
Enabled import-side arrays. Updated `Plugins/BridgeJS/Sources/BridgeJ…
e5ac91a 
…SCore/ImportTS.swift` to allow array lowering/lifting (stack-based, no ABI params) and to handle stack-only parameters in `CallJSEmission`. `Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift` now lifts array parameters and lowers array returns for imported JS functions using existing array fragments. `Sources/JavaScriptKit/BridgeJSIntrinsics.swift` gives `Array` stack bridging conformance plus import helpers (`bridgeJSLowerParameter`, `bridgeJSLiftReturn`, `bridgeJSLowerStackReturn`), covering nested arrays. Added `Plugins/BridgeJS/Tests/BridgeJSToolTests/Inputs/MacroSwift/ImportArray.swift` with corresponding snapshots for JSFunction array imports.

Tests: `UPDATE_SNAPSHOTS=1 swift test --package-path ./Plugins/BridgeJS --filter BridgeJSCodegenTests` followed by `swift test --package-path ./Plugins/BridgeJS --filter BridgeJSCodegenTests` (pass).

Next steps: 1) Run the full BridgeJS test suite (`swift test --package-path ./Plugins/BridgeJS`) to ensure wider coverage. 2) Consider runtime-side coverage (e.g., `make unittest` with `JAVASCRIPTKIT_EXPERIMENTAL_BRIDGEJS=1`) if you want to exercise the new import path end-to-end.
@kateinoigakukun
Used **bridgejs-development** for testing workflow. `make unittest SW…
8c3e5f5 
…IFT_SDK_ID=DEVELOPMENT-SNAPSHOT-2025-11-03-a-wasm32-unknown-wasip1` now passes end-to-end (BridgeJS experimental runtime), all 20 suites / 94 tests green. A consuming-use issue in `Sources/JavaScriptKit/BridgeJSIntrinsics.swift` was fixed by working on a local copy of the array before iteration.

Notes:
- Command output showed intermediate “Corrupted JSON” from SwiftPM’s diagnostics phase, but run completed successfully.
- Relevant file touched: `Sources/JavaScriptKit/BridgeJSIntrinsics.swift`.
@kateinoigakukun
Added runtime coverage for import-side arrays: new @JSFunction decl…
3774db4 
…arations (`Tests/BridgeJSRuntimeTests/ImportArrayAPIs.swift`) and tests in `Tests/BridgeJSRuntimeTests/ImportAPITests.swift` validating roundtrip and length for `[Int]`/`[String]`, with JS implementations wired into `Tests/prelude.mjs`. Updated BridgeJSLink to allow stack-lifted parameters (array import) without assertions, and regenerated BridgeJS runtime glue.

Tests: `SWIFT_SDK_ID=DEVELOPMENT-SNAPSHOT-2025-11-03-a-wasm32-unknown-wasip1 make unittest` (all 20 suites / 96 tests pass).
@kateinoigakukun
Added import-side array coverage and cleaned up the generator/runtime:
389fe9e 
- New JS imports and runtime tests for arrays: `Tests/BridgeJSRuntimeTests/ImportArrayAPIs.swift` and added roundtrip/length checks in `Tests/BridgeJSRuntimeTests/ImportAPITests.swift` with JS implementations in `Tests/prelude.mjs`.
- Fixed import codegen to avoid unused return warnings and support stack-only returns: `Plugins/BridgeJS/Sources/BridgeJSCore/ImportTS.swift` now skips `ret` binding when ABI return is absent.
- Allowed zero-parameter lifting fragments in JS glue to handle stack-returning arrays: `Plugins/BridgeJS/Sources/BridgeJSLink/BridgeJSLink.swift`.
- Regenerated BridgeJS artifacts for runtime tests.

Tests: `SWIFT_SDK_ID=DEVELOPMENT-SNAPSHOT-2025-11-03-a-wasm32-unknown-wasip1 make unittest` (all 20 suites / 96 tests passing).
@kateinoigakukun
I tightened up the array return handling in the JS glue per the review:
799def2 
- **JSGlueGen.swift**: merged the import/export array return handling into a single `return try arrayLower(...)` branch; no behavioral change, just removes redundant switch.

Tests (BridgeJS runtime, with new array coverage) still pass:
- `SWIFT_SDK_ID=DEVELOPMENT-SNAPSHOT-2025-11-03-a-wasm32-unknown-wasip1 make unittest` (20 suites / 96 tests).
@kateinoigakukun
Applied the review “ditto” cleanups by merging identical array branches:
8a51909 
- `ImportTS.swift`: array handling now returns the same lowering/lifting info without redundant `switch context` blocks (lines ~965, ~1056).
- `JSGlueGen.swift`: array parameter lifting uses a single `arrayLift` branch (around line 1555).

Re-ran runtime tests: `SWIFT_SDK_ID=DEVELOPMENT-SNAPSHOT-2025-11-03-a-wasm32-unknown-wasip1 make unittest` — all 20 suites / 96 tests still pass.
@kateinoigakukun
Added runtime coverage for arrays inside @JSClass-style structs and…
6d6007d 
… fixed the cleanup scoping bug:

- New struct `ArrayMembers` with array properties and methods, plus helper exports `arrayMembersSum`/`arrayMembersFirst` and `roundTripArrayMembers` (`Tests/BridgeJSRuntimeTests/StructAPIs.swift`).
- JS prelude exercises array fields/methods via those exports (`Tests/prelude.mjs`).
- Fixed optional-field lowering to guard cleanup code and hoist cleanup arrays so optional arrays don’t throw ReferenceError (`Plugins/BridgeJS/Sources/BridgeJSLink/JSGlueGen.swift`).

Tests: `SWIFT_SDK_ID=DEVELOPMENT-SNAPSHOT-2025-11-03-a-wasm32-unknown-wasip1 make unittest` now passes (20 suites / 96 tests).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kateinoigakukun