Fix ios tests - #4
Merged
Merged
Conversation
Return the raw wire value array from iOS host callbacks instead of collapsing empty and single-value results. This keeps Swift trampoline handling consistent and avoids fragile NSNumber marshalling on the single-value path.
Add `nativescript` as a workspace dev dependency so `ns` is available from the local toolchain, and update the AGENTS docs to consistently run it via `npx ns`. This avoids relying on a global NativeScript install and prevents the macOS permission issues called out in the test instructions.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Notice: It looks like you're using hustcer/deepseek-review, but the CHAT_TOKEN hasn't been configured in your repo's Variables/Secrets. Please ensure this token is set for proper functionality. For step-by-step guidance, refer to the CHAT_TOKEN Config section of README.
The gradle/actions/setup-gradle wrapper validation scans the entire workspace and flags the jar bundled inside node_modules/nativescript (an npm dependency of the test app). That jar's checksum is not in Gradle's published list, so the step fails. Our wrapper is committed in the repo; the npm package's jar is not ours to validate.
Pin Node.js 24.18.1, Java temurin-21.0.2, and Gradle 9.6.1 — matching the versions used in CI (actions/setup-node, setup-java, and the Android project's Gradle wrapper). Swift/Xcode and Kotlin/NDK are noted as managed externally.
There was a problem hiding this comment.
Notice: It looks like you're using hustcer/deepseek-review, but the CHAT_TOKEN hasn't been configured in your repo's Variables/Secrets. Please ensure this token is set for proper functionality. For step-by-step guidance, refer to the CHAT_TOKEN Config section of README.
Two fixes for the 'call call_transform_i32: returned null' error that broke host imports with return values on iOS: 1. Rename JS override from invoke() to invokeWithArg(). NativeScript maps the ObjC selector invoke: (single unnamed argument) to the JS method name invokeWithArg, not invoke. With the wrong name the base class's invoke was called instead, returning nil. The trampoline converted nil→[] then 0!=nRets for value-returning imports, causing a wasm trap and the 'returned null' error. Void imports happened to work because 0==0. 2. Retain ObjC callback objects on the JS side via a hostCallbacks array on IosRuntime. The NativeScript bridge ties ObjC object lifetime to the JS wrapper—without a JS reference the GC may deallocate the callback object even though Swift's HostContext holds a strong reference.
macOS 15+ restricts sandbox-exec to Apple-signed processes, so SwiftPM's manifest sandboxing fails with 'Operation not permitted' when running swift test from a terminal. The --disable-sandbox flag tells SwiftPM to skip manifest sandboxing — harmless on CI where the restriction doesn't apply, and required for local development.
The DeepSeek code review action was triggering on every PR open/reopen/ synchronize and issue comment. Switch to workflow_dispatch so it only runs when manually triggered from the Actions tab.
Three coordinated changes across the Swift, TypeScript, and test layers to fix the iOS host-import bug (call_transform_i32: returned null): Swift (NSCWasm3.swift): - Add 'dynamic' to invoke() so NativeScript's .extend() override is reachable through objc_msgSend, not just the Swift vtable. - Narrow invoke() return type from Any? to NSArray?. NativeScript converts a JS return value using the declared ObjC type; an untyped id makes it wrap a JS array as an NSDictionary keyed by index. - Simplify the trampoline: the callback now always returns [Any] instead of Any?, removing the nil/single/array switch. Swift tests (NSCWasm3Tests.swift): - Add testHostImportOverriddenThroughTheObjCRuntime — creates an ObjC subclass at runtime with class_addMethod to prove dispatch works through objc_msgSend, not just the Swift vtable. - Update existing callbacks to return NSArray per the new signature. TypeScript (wasm3-ios.ts): - Add toNsArray() to build real NSMutableArray objects. A plain JS array returned from a JS-implemented override is silently dropped by the NativeScript bridge. - Add withErrorRef() helper: NativeScript surfaces NSError** as an extra argument, and failures return null + fill the reference. Without passing a reference, every error looks like a bare null. - Refactor all throwing calls (loadModule, findFunction, getGlobal, setGlobal, readMemory, writeMemory, call, linkHostFunction) to use withErrorRef() so wasm3 error messages survive the bridge. - Extract hostCallbacks.push() outside the try block — a callback must be retained even if linkHostFunction itself fails. Tests (wasm3.spec.ts): - Update iOS fake to mirror the NSError reference protocol. - Add NSMutableArray fake. - Add spec: 'reads the wasm3 message out of the NSError reference'.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.