[dart:js_interop] Unhelpful error message about what types are allowed when trying to use static interop

[annagrin]

I am migrating DWDS injected client code to package:web. I've encountered a cryptic error message on one of the changes:

Old code

void _listenForDebugExtensionAuthRequest(AppInfo appInfo) {
  window.addEventListener(
    'message',
    ((event) async {
       // Do something here
    }).toPromise
    },
  );
}

New code

void _listenForDebugExtensionAuthRequest(AppInfo appInfo) {
  window.addEventListener(
    'message',
    ((event) => _handleMessage(event, appInfo)).toJS, // This causes the error below (I think it needs `event` type?)
  );
}

void _handleMessage(Event event, AppInfo appInfo) {
  // Do something here
}

Expected

Some information on error location and nature, and ideally on how to fix it.

Actual

org-dartlang-app:///web/client.dart@8916+1:
Error: Type 'dynamic' is not a valid type for external `dart:js_interop` APIs. The only valid types are: @staticInterop types, JS types from `dart:js_interop`, void, bool, num, double, int, String, and any extension type that erases to one of these types.
Error: Compilation failed.

(note that line 8916 does not exist client.dart)

[srujzs]

You're right that event in ((event) => _handleMessage(event, appInfo)).toJS is the culprit here. event here is implicitly dynamic, which isn't allowed. Making this JSAny? or any of the other compatible types will make this work.

In terms of reporting, it's a bit complex to do better. Currently, we report the error on the node in which the error occurred in (the call to toJS here). Reporting information on the type itself is hard because:

1. Types don't have location information. I think the CFE uses offsets and such to work around this.
2. The function on which we call toJS might just be some variable, so we might not even know where the function it corresponds to is.

We may be able to do better by rewording the error to explicitly refer to either the return type or the parameter type to which it corresponds. For parameters, we can't use the name because of issue #2, but we may be able to use the position in the type to make it easier e.g. This function's *2nd parameter* has type dynamic, which isn't allowed.. This error is used for external members too e.g. external int f(int a, dynamic b);, so we can do something similar there. In that case, we do have the parameter names and perhaps offsets, so we can maybe do even better.

[annagrin]

Thanks @srujzs , it would be great to resolve this because currently I get this error quite often and need to binary-search the file to find the problematic line. Btw, what is the location org-dartlang-app:///web/client.dart@8916+1: referring to?

[srujzs]

I think there's something off about the location information or the way it's displayed. Here's where we report the info:

sdk/pkg/_js_interop_checks/lib/js_interop_checks.dart

Line 922 in 706d105

void _reportIfNotAllowedExternalType(

, where the node is a StaticInvocation that comes from here:

sdk/pkg/_js_interop_checks/lib/js_interop_checks.dart

Line 405 in 706d105

if (target == _functionToJSTarget) {

. I don't know why that info would bubble up this way. Do other CFE errors report differently for you as well or is it just this one?

[sigmundch]

Interesting, the @8916+1 is a byte offset on the file. Jumping to offset 8916 (not sure if that's expose in vscode, though) should point to the right location.

Error reporting from the CFE (and similarly from dart2js) starts with offsets and converts them to line/column positions if the source-file contents are available during that step of the compilation process. That also is necessary to display context with the error message (e.g. the text snippet with an underlined squiggle line). The setup may depend on what compiler is being invoked in order to run the JSInterop transformer.

@annagrin - what are the commands you use to compile client.dart that trigger the error? I'm curious which compiler is the one triggering the error first, and we can start looking from there.

[annagrin]

@srujzs @sigmundch I build using dart build+build_web_compilers with dart2js. I do not see those errors on VSCode as they only show during the actual build and not in the analyzer.

You can try it out by running dart run build_runner build in webdev/dwds directory (after cloning webdev).

Actually, other CFE errors report twice and the second report has the same problem, for example, if I make a syntax error:

[INFO] Reading cached asset graph completed, took 177ms
[INFO] Checking for updates since last build completed, took 527ms
[SEVERE] built_value_generator:built_value on web/reloader/require_restarter.dart:

This builder requires Dart inputs without syntax errors.
However, asset:dwds/web/reloader/require_restarter.dart (or an existing part) contains the following errors.
require_restarter.dart:123:7: Expected to find ','.

Try fixing the errors and re-running the build.

[INFO] build_web_compilers:entrypoint on web/client.dart:Running `dart compile js` with -O1 --no-source-maps --libraries-spec=/Users/annagrin/.dart-sdks/3.3.0-197.0.dev/lib/libraries.json --packages=org-dartlang-app:///.dart_tool/package_config.json --multi-root-scheme=org-dartlang-app --multi-root=/private/var/folders/v9/232dwc2x0t113h5qhxkkgcjm00p2zc/T/scratch_spaceLTfI0b/ -oweb/client.dart.js org-dartlang-app:///web/client.dart
[SEVERE] build_web_compilers:entrypoint on web/client.dart:
ExitCode:1
StdOut:
org-dartlang-app:///web/reloader/require_restarter.dart@3416+12:
Error: Expected ',' before this.
Error: Compilation failed.

StdErr:

[INFO] Running build completed, took 4.6s
[INFO] Caching finalized dependency graph completed, took 225ms
[SEVERE] Failed after 4.8s

Btw looks like the first error is reported by the built value generator, before the actual build (to generate classes and serializers we use in network comunications), and the second one by the actual build with dart2js.

[annagrin]

/cc @fishythefish @rakudrama

[fishythefish]

I believe the byte offset printing comes from here.

[biggs0125]

Yes Dart2JS will emit this offset format when the CFE does not register the source code for a file. Normally the CFE will call a hook in Dart2JS with the URI and the associated source code:
https://github.com/dart-lang/sdk/blob/main/pkg/compiler/lib/src/source_file_provider.dart#L60

We have a flag to disable registering those URIs (to save memory by not having all the source code in memory) but that should be disabled by default. So I would guess that the CFE is filtering out the source bytes for that package for some reason... perhaps if its bundled as part of the SDK.

[sigmundch]

This seems to be directly related to the use of the --multi-root option from build_web_compilers. So the offset reporting is a dart2js issue.

For example, take this program:

repro.dart:

import 'dart:js_interop';

main() {
  ((a, int b) {}).toJS;
}

The reported error changes depending on how we invoke dart2js:

> dart compile js repro.dart
repro.dart:4:19:
Error: Type 'dynamic' is not a valid type in the signature of `dart:js_interop` external APIs or APIs converted via `toJS`. The only valid types are: JS types from `dart:js_interop`, @staticInterop types, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop type.
  ((a, int b) {}).toJS;
                  ^
Error: Compilation failed.

In constast:

> dart compile js -O1 --multi-root-scheme=foo --multi-root=$PWD/ foo:///repro.dart
foo:///repro.dart@54+1:
Error: Type 'dynamic' is not a valid type in the signature of `dart:js_interop` external APIs or APIs converted via `toJS`. The only valid types are: JS types from `dart:js_interop`, @staticInterop types, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop type.
Error: Compilation failed.

Note that our alternative approach with bazel-paths works properly:

dart compile js -O1 --bazel-paths=$PWD/ file:///bazel-root/repro.dart                                                                                                                                                                                                                              
/bazel-root/repro.dart:4:19:
Error: Type 'dynamic' is not a valid type in the signature of `dart:js_interop` external APIs or APIs converted via `toJS`. The only valid types are: JS types from `dart:js_interop`, @staticInterop types, void, bool, num, double, int, String, extension types that erases to one of these types, or a type parameter that is bound to a static interop type.
  ((a, int b) {}).toJS;
                  ^
Error: Compilation failed.

@srujzs - I agree. In terms of error reporting, I think we can be more precise and guide users a bit better (as you suggest, at least by pointing them to the right argument/return type depending on the scenario.)

[sigmundch]

@biggs0125 - I looked more closely at the discrepancy, and it's sort of by accident that this works with bazel-paths. Our recent changes to the caching logic made it so that we don't prime the cache until after we are done with the CFE phase (in load-kernel). However, here the diagnostic is being produced before that point in time, during a transformer that is running in the middle of the CFE phase.

Some options to consider:
(a) we could move this logic to 0b instead of running during the CFE phase.
(b) we could change the cache registration to be done on-demand during the CFE phase instead
(c) we could try the fallback we have with bazel-paths also with multi-root (it will likely read the contents properly if we tried)
(d) see if we can change what we use for diagnostics.

Re (d), I wonder if other regular CFE diagnostics are broken too, if they are not, then this may be a specific issue of how transformers are directing diagnostics directly through dart2js and not through the CFE. If however we are using the same diagnostics, it's possible that regular CFE errors are also impaired. In that case, we should probably look into (b).