Skip to content

Conversation

@ArcadeMode
Copy link
Contributor

@ArcadeMode ArcadeMode commented Jan 26, 2026

Fixes #97380.
Fixes #123706

Implementation is functional for CoreCLR and Mono. Added import and export tests to demonstrate.

How do I go about updating the docs? (and which version might see these changes for that matter, net10/11?)

@pavelsavara
Copy link
Member

@ArcadeMode feel free to ping me on your PRs, so that I can label it properly.

@ArcadeMode ArcadeMode marked this pull request as ready for review January 27, 2026 22:55
Copilot AI review requested due to automatic review settings January 27, 2026 22:55
@ArcadeMode
Copy link
Contributor Author

@pavelsavara to verify that i am following contribution guidelines: I have already changed and implemented an API change, is it okay to ask for API review in PR like this?

i.e. would a similar approach suffice for #97381 or should this be done through the issue or otherwise?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request adds marshaling support for float[], Span<float>, and ArraySegment<float> between C# and JavaScript in the browser WASM interop layer, addressing issue #97380.

Changes:

  • Added MemoryViewType.Single enum value and corresponding Float32Array handling
  • Implemented C# marshaling methods for float arrays, spans, and array segments
  • Updated TypeScript marshaling code to handle Single type in both native and mono implementations
  • Added comprehensive test coverage for JSImport and JSExport scenarios with float types
  • Updated code generator to recognize Single type patterns

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshaled-types.ts Added MemoryViewType.Single enum and Float32Array view creation
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal.ts Added Single to elementSizeMap with 4-byte size
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-js.ts Implemented Single type marshaling for arrays, spans, and array segments
src/native/libs/System.Runtime.InteropServices.JavaScript.Native/interop/marshal-to-cs.ts Implemented Single type unmarshaling and view type checking
src/mono/browser/runtime/marshal.ts Added Single to array_element_size function and MemoryViewType enum, refactored to if-else chains
src/mono/browser/runtime/marshal-to-js.ts Implemented Single type marshaling for arrays, spans, and array segments in mono runtime
src/mono/browser/runtime/marshal-to-cs.ts Implemented Single type unmarshaling and view type checking in mono runtime
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Single.cs Added ToJS/ToManaged methods for float[], Span, and ArraySegment
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/Marshaling/JSMarshalerArgument.Object.cs Added float[] handling in ToJS method for object parameters
src/libraries/System.Runtime.InteropServices.JavaScript/src/System/Runtime/InteropServices/JavaScript/JSMarshalerType.cs Updated validation methods to accept Single type in arrays, spans, and segments; removed extraneous blank line
src/libraries/System.Runtime.InteropServices.JavaScript/ref/System.Runtime.InteropServices.JavaScript.cs Added public API surface for float[], Span, and ArraySegment marshaling
src/libraries/System.Runtime.InteropServices.JavaScript/gen/JSImportGenerator/JSGeneratorFactory.cs Added generator patterns for Single type arrays, spans, and array segments
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JavaScriptTestHelper.cs Added test helper methods for Single type JSImport and JSExport scenarios
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSInteropTestBase.cs Added MarshalSingleArrayCases test data
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSImportTest.cs Added comprehensive tests for float arrays, spans, and array segments; improved variable naming in Double/Int32 tests
src/libraries/System.Runtime.InteropServices.JavaScript/tests/.../JSExportTest.cs Added JSExport tests for Span and ArraySegment

@pavelsavara
Copy link
Member

is it okay to ask for API review in PR like this?

The process is described here https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md

You need to create dedicated issue according to the issue template for API suggestion.

You can also look at successfully approved APIs

@ArcadeMode
Copy link
Contributor Author

is it okay to ask for API review in PR like this?

The process is described here https://github.com/dotnet/runtime/blob/main/docs/project/api-review-process.md

You need to create dedicated issue according to the issue template for API suggestion.

You can also look at successfully approved APIs

Thanks for the pointers. I have created #123706 with the proposal

@pavelsavara
Copy link
Member

pavelsavara commented Jan 28, 2026

test for JS Array values of wrong type or JS number (double) above float.max would be great

@ArcadeMode
Copy link
Contributor Author

test for JS Array values of wrong type or JS number (double) above float.max would be great

I have created #123731 to resolve issues with type assertions for array elements. Here I would only end up adding demonstration that the float would overflow (as demonstrated for int32). I hope I can address that issue separately.

@pavelsavara pavelsavara added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jan 29, 2026
@pavelsavara
Copy link
Member

The code looks good. I added NO-MERGE label until we pass the API review.

Copilot AI review requested due to automatic review settings January 29, 2026 17:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.

Comment on lines +527 to +529
var expectedBytes = stackalloc float[] { 0, 1, -1, float.Pi, 42, float.MaxValue, float.MinValue, float.NaN, float.PositiveInfinity, float.NegativeInfinity };
Span<float> expected = new Span<float>(expectedBytes, 10);
Assert.True(Unsafe.AsPointer(ref expected.GetPinnableReference()) == expectedBytes);
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable name expectedBytes is misleading since it contains float data, not bytes. For consistency with other test methods in this file (e.g., expectedInts on line 558, expectedDoubles on line 572, and expectedFloats on line 586), this variable should be renamed to expectedFloats.

Suggested change
var expectedBytes = stackalloc float[] { 0, 1, -1, float.Pi, 42, float.MaxValue, float.MinValue, float.NaN, float.PositiveInfinity, float.NegativeInfinity };
Span<float> expected = new Span<float>(expectedBytes, 10);
Assert.True(Unsafe.AsPointer(ref expected.GetPinnableReference()) == expectedBytes);
var expectedFloats = stackalloc float[] { 0, 1, -1, float.Pi, 42, float.MaxValue, float.MinValue, float.NaN, float.PositiveInfinity, float.NegativeInfinity };
Span<float> expected = new Span<float>(expectedFloats, 10);
Assert.True(Unsafe.AsPointer(ref expected.GetPinnableReference()) == expectedFloats);

Copilot uses AI. Check for mistakes.
[return: JSMarshalAs<JSType.Number>]
internal static partial double? store_DoubleArray([JSMarshalAs<JSType.Array<JSType.Number>>] double[]? value, [JSMarshalAs<JSType.Number>] int index);

[JSImport("echo1", "JavaScriptTestHelper")]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add few signatures without JSMarshalAs annotations. They should work fine too.

@pavelsavara
Copy link
Member

pavelsavara commented Jan 29, 2026

Please add few float signatures so that the generated code appears in src/libraries/System.Runtime.InteropServices.JavaScript/tests/JSImportGenerator.UnitTest/Compiles.cs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arch-wasm WebAssembly architecture area-System.Runtime.InteropServices.JavaScript community-contribution Indicates that the PR has been added by a community member NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) os-browser Browser variant of arch-wasm

Projects

None yet

2 participants