Skip to content

Conversation

@osa1
Copy link
Member

@osa1 osa1 commented Aug 26, 2025

In a declaration like

@JS('JSON')
extension type _JSON._(JSObject _) implements JSObject {
  @JS('JSON.stringify')
  external static JSString _stringify(JSObject value);
}

When I call _JSON._stringify(...), dart2js calls JSON.stringify, but dart2wasm calls JSON.JSON.stringify, which is wrong.

Reading 1 it looks like the annotation JSON.stringify should just be stringify. If I do that the code works with both dart2wasm and dart2js.

(I've reported dart2wasm's handling of the annotations above and we'll work on a fix for it on dart2wasm.)

This change is not tested on the CI because when compiling with dart2wasm we use the JS decoder library used by the VM, which doesn't use JS interop. I tested this manually with the patch:

diff --git a/protobuf/lib/src/protobuf/json/json.dart b/protobuf/lib/src/protobuf/json/json.dart
index 05d1ac1..70b6a7c 100644
--- a/protobuf/lib/src/protobuf/json/json.dart
+++ b/protobuf/lib/src/protobuf/json/json.dart
@@ -13,7 +13,7 @@ import '../utils.dart';
 // Use json_vm.dart with VM and dart2wasm, json_web.dart with dart2js.
 // json_web.dart uses JS interop for parsing, and JS interop is too slow on
 // Wasm. VM's patch performs better in Wasm.
-export 'json_vm.dart' if (dart.library.html) 'json_web.dart';
+export 'json_vm.dart' if (dart.library.js_interop) 'json_web.dart';

 Map<String, dynamic> writeToJsonMap(FieldSet fs) {
   dynamic convertToMap(dynamic fieldValue, int fieldType) {

In a declaration like

    @js('JSON')
    extension type _JSON._(JSObject _) implements JSObject {
      @js('JSON.stringify')
      external static JSString _stringify(JSObject value);
    }

When I call `_JSON._stringify(...)`, dart2js calls `JSON.stringify`, but
dart2wasm calls `JSON.JSON.stringify`, which is wrong.

Reading [1] it looks like the annotation `JSON.stringify` should just be
`stringify`. If I do that the code works with both dart2wasm and
dart2js.

(I've reported dart2wasm's handling of the annotations above and we'll
work on a fix for it on dart2wasm.)

[1]: https://dart.dev/interop/js-interop/usage#js
@github-actions
Copy link

Package publishing

Package Version Status Publish tag (post-merge)
package:protobuf 5.0.0 (error) pubspec version (5.0.0) and changelog (5.0.0-wip) don't agree
package:protoc_plugin 23.0.0 ready to publish protoc_plugin-v23.0.0

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

@osa1 osa1 merged commit 9939965 into google:master Aug 26, 2025
12 checks passed
@osa1 osa1 deleted the fix_js_annotations branch August 26, 2025 09:49
Spiritus2424 pushed a commit to EXFO/protobuf.dart that referenced this pull request Aug 28, 2025
In a declaration like

    @js('JSON')
    extension type _JSON._(JSObject _) implements JSObject {
      @js('JSON.stringify')
      external static JSString _stringify(JSObject value);
    }

When I call `_JSON._stringify(...)`, dart2js calls `JSON.stringify`, but dart2wasm calls `JSON.JSON.stringify`, which is wrong.

Reading [1] it looks like the annotation `JSON.stringify` should just be `stringify`. If I do that the code works with both dart2wasm and dart2js.

(I've reported dart2wasm's handling of the annotations above and we'll work on a fix for it on dart2wasm.)

This change is not tested on the CI because when compiling with dart2wasm we use the JS decoder library used by the VM, which doesn't use JS interop. I tested this manually with the patch:

    diff --git a/protobuf/lib/src/protobuf/json/json.dart b/protobuf/lib/src/protobuf/json/json.dart
    index 05d1ac1..70b6a7c 100644
    --- a/protobuf/lib/src/protobuf/json/json.dart
    +++ b/protobuf/lib/src/protobuf/json/json.dart
    @@ -13,7 +13,7 @@ import '../utils.dart';
     // Use json_vm.dart with VM and dart2wasm, json_web.dart with dart2js.
     // json_web.dart uses JS interop for parsing, and JS interop is too slow on
     // Wasm. VM's patch performs better in Wasm.
    -export 'json_vm.dart' if (dart.library.html) 'json_web.dart';
    +export 'json_vm.dart' if (dart.library.js_interop) 'json_web.dart';

     Map<String, dynamic> writeToJsonMap(FieldSet fs) {
       dynamic convertToMap(dynamic fieldValue, int fieldType) {

[1]: https://dart.dev/interop/js-interop/usage#js
Spiritus2424 pushed a commit to EXFO/protobuf.dart that referenced this pull request Aug 28, 2025
In a declaration like

    @js('JSON')
    extension type _JSON._(JSObject _) implements JSObject {
      @js('JSON.stringify')
      external static JSString _stringify(JSObject value);
    }

When I call `_JSON._stringify(...)`, dart2js calls `JSON.stringify`, but dart2wasm calls `JSON.JSON.stringify`, which is wrong.

Reading [1] it looks like the annotation `JSON.stringify` should just be `stringify`. If I do that the code works with both dart2wasm and dart2js.

(I've reported dart2wasm's handling of the annotations above and we'll work on a fix for it on dart2wasm.)

This change is not tested on the CI because when compiling with dart2wasm we use the JS decoder library used by the VM, which doesn't use JS interop. I tested this manually with the patch:

    diff --git a/protobuf/lib/src/protobuf/json/json.dart b/protobuf/lib/src/protobuf/json/json.dart
    index 05d1ac1..70b6a7c 100644
    --- a/protobuf/lib/src/protobuf/json/json.dart
    +++ b/protobuf/lib/src/protobuf/json/json.dart
    @@ -13,7 +13,7 @@ import '../utils.dart';
     // Use json_vm.dart with VM and dart2wasm, json_web.dart with dart2js.
     // json_web.dart uses JS interop for parsing, and JS interop is too slow on
     // Wasm. VM's patch performs better in Wasm.
    -export 'json_vm.dart' if (dart.library.html) 'json_web.dart';
    +export 'json_vm.dart' if (dart.library.js_interop) 'json_web.dart';

     Map<String, dynamic> writeToJsonMap(FieldSet fs) {
       dynamic convertToMap(dynamic fieldValue, int fieldType) {

[1]: https://dart.dev/interop/js-interop/usage#js
copybara-service bot pushed a commit to dart-lang/sdk that referenced this pull request Aug 28, 2025
Revisions updated by `dart tools/rev_sdk_deps.dart`.

ecosystem (https://github.com/dart-lang/ecosystem/compare/68ff911..fd28be2):
  fd28be2  2025-08-27  Moritz  Dont show warnings by default (dart-lang/ecosystem#365)

http (https://github.com/dart-lang/http/compare/6656f15..ef05b37):
  ef05b37  2025-08-27  Brian Quinlan  [cupertino_http]: Switch to ffigen 19.1.0 (dart-lang/http#1811)

i18n (https://github.com/dart-lang/i18n/compare/6a8f9c7..9a211d1):
  9a211d14  2025-08-27  Moritz  [package:intl4x] `DateTime` formatting redesign (dart-lang/i18n#1003)

protobuf (https://github.com/dart-lang/protobuf/compare/6e9c9f4..765ba8a):
  765ba8a  2025-08-28  Ömer Sinan Ağacan  Fix non-proto3 JSON format double decoding (google/protobuf.dart#1043)
  703bf7c  2025-08-27  Ömer Sinan Ağacan  Compile dart2wasm benchmarks with --no-strip-wasm (google/protobuf.dart#1044)
  9939965  2025-08-26  Ömer Sinan Ağacan  Fix JS interop annotations (google/protobuf.dart#1042)
  da354a2  2025-08-22  Ömer Sinan Ağacan  Update benchmark builder to use -O2 with dart2wasm, add output dir to gitignore (google/protobuf.dart#1041)
  41eba19  2025-08-22  Ömer Sinan Ağacan  Add sinks to benchmarks to prevent smart tools eliminating benchmarked code (google/protobuf.dart#1040)
  3ccc8ab  2025-08-19  dependabot[bot]  Bump actions/checkout from 3.5.3 to 4.2.2 (google/protobuf.dart#1036)
  6bce9d7  2025-08-19  Ömer Sinan Ağacan  Fix unknown enum handling (google/protobuf.dart#853)
  19e5a25  2025-08-15  Ömer Sinan Ağacan  Implement deepCopy as copying, instead of merge (google/protobuf.dart#742)

web (https://github.com/dart-lang/web/compare/ee9733f..e7895bd):
  e7895bd  2025-08-27  Nikechukwu  [interop] Add Support for Declaration Merging and Overloading (dart-lang/web#449)

webdev (https://github.com/dart-lang/webdev/compare/9724fea..a7d3d2f):
  a7d3d2fc  2025-08-27  Ben Konyi  [ DWDS ] Fix issue where null was repeatedly sent to connected clients (dart-lang/webdev#2678)
  e3009dfb  2025-08-27  Nate Biggs  Remove stale calls to dartSdkIsAtLeast. (dart-lang/webdev#2677)
  e0d58082  2025-08-26  Nate Biggs  Fix expectation in test accessing private field. (dart-lang/webdev#2676)

Change-Id: Ia98a8f065830865f6922ca032939380683645d72
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/447640
Reviewed-by: Konstantin Shcheglov <[email protected]>
Commit-Queue: Konstantin Shcheglov <[email protected]>
Auto-Submit: Devon Carew <[email protected]>
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