Skip to content
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.

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

dart2js: toString() breaks for the JS Symbol type #53106

Closed
nex3 opened this issue Aug 3, 2023 · 6 comments
Closed

dart2js: toString() breaks for the JS Symbol type #53106

nex3 opened this issue Aug 3, 2023 · 6 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-ddc-discrepancy When dev and production compilations have different semantics dart2js-interceptors dart2js-runtime-library Issues related to runtime library for dart2js (sdk/lib/_internal/js_runtime) P2 A bug or feature request we're likely to work on web-dart2js web-js-interop Issues that impact all js interop

Comments

@nex3
Copy link
Member

nex3 commented Aug 3, 2023

Reproduction:

import 'package:js/js.dart';

@JS("Symbol")
class JSSymbol {}

@JS("Symbol")
external JSSymbol createJSSymbol(String name);

void main() {
  createJSSymbol("foo").toString();
}

Compiling to JS with dart2js (Dart 3.0.7) and running in the browser throws method not found: 'toString$0' (J.getInterceptor$(...).toString$0 is not a function).

@rakudrama rakudrama added web-dart2js area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-runtime-library Issues related to runtime library for dart2js (sdk/lib/_internal/js_runtime) dart2js-interceptors dart2js-ddc-discrepancy When dev and production compilations have different semantics labels Aug 3, 2023
@rakudrama
Copy link
Member

There is likely a similar issue with the other primitive JavaScript type introduced since ES5: typeof x == "bigint".
/cc @nshahan What does DDC do here?

@nex3
Copy link
Member Author

nex3 commented Aug 3, 2023

I can confirm that this happens for BigInt as well.

@sigmundch sigmundch added the P2 A bug or feature request we're likely to work on label Aug 4, 2023
@rakudrama
Copy link
Member

rakudrama commented Aug 8, 2023

I want to keep open the option of using JavaScript's BigInt for Dart's BigInt.
That would mean having full special-case interceptors like we do for numbers and bools.
What is tricky here is that the primitive JavaScript objects number, string and boolean and now bigint and symbol are not really classes. This means much deeper integration with how dart2js models these things including generating special cases in getInterceptor.

@srujzs - One thing I am not clear on is how @JS() interop and the new static interop is supposed to call the right methods (toString, hashCode etc) for intercepted classes like Uint8Array which have a first-class @Native interpretation, and these primitive objects which are not even JavaScript Objects.

@nshahan
Copy link
Contributor

nshahan commented Aug 8, 2023

Currently this crashes in DDC as well when it tries to call the Dart .toString() member on the symbol. Coincidentally the changes I'm about to land to the calling conventions of the Dart Core Object APIs will fix the crash and instead it will call the native JavaScript .toString() member which returns 'Symbol(foo)'.

@srujzs
Copy link
Contributor

srujzs commented Aug 8, 2023

One thing I am not clear on is how @JS() interop and the new static interop is supposed to call the right methods (toString, hashCode, etc) for intercepted classes like Uint8Array which have a first-class @Native interpretation, and these primitive objects which are not even JavaScript Objects.

We really haven't specified a behavior here beyond what occurs at runtime (we should). In the case of @staticInterop where it can be typed_data or dart:html objects, we should be getting their corresponding interceptors at runtime and then calling the Object methods from there. For the dart:js_interop types (of which, there are primitives), I'd expect the same. Are you implying they should be different than that behavior?

We do want to expose Symbol and BigInt through dart:js_interop but we should probably wait until we have interceptors for them.

@srujzs srujzs added the web-js-interop Issues that impact all js interop label Aug 8, 2023
copybara-service bot pushed a commit that referenced this issue Aug 10, 2023
The interceptors provide a Dart `toString` method that uses the JavaScript `toString` method.

Issue: #53106

Change-Id: I1cf1df9e24fb4fd2d79679f1f014f39f083be7e9
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/319563
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Stephen Adams <sra@google.com>
@srujzs
Copy link
Contributor

srujzs commented Aug 23, 2023

@nex3 Both DDC and dart2js now intercept Symbols and BigInts as separate interceptors. In DDC, this used to be LegacyJavaScriptObject, but we never had an interceptor in dart2js. This change means two things:

  1. toString should now work correctly by calling the external toString/String(#) method.
  2. You can't use package:js classes to interop with these types as they're not a subtype of JavaScriptObject but rather Interceptor. You couldn't really do this with dart2js before, but DDC allowed it because of how it intercepted unknown types. We've introduced types in dart:js_interop called JSSymbol and JSBigInt to use instead. If you opt into the extension types experiment, you can interop using those as your representation type. Another alternative is js_util of course.

@srujzs srujzs closed this as completed Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-ddc-discrepancy When dev and production compilations have different semantics dart2js-interceptors dart2js-runtime-library Issues related to runtime library for dart2js (sdk/lib/_internal/js_runtime) P2 A bug or feature request we're likely to work on web-dart2js web-js-interop Issues that impact all js interop
Projects
None yet
Development

No branches or pull requests

5 participants