search

dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.22k stars 1.57k forks source link

JS staticInterop crashes on factory constructor #49778

Closed matanlurey closed 1 year ago

matanlurey commented 2 years ago

Minimal reproduction:

import 'package:js/js.dart';

@JS()
@staticInterop
abstract class WebGLRenderingContext {
  factory WebGLRenderingContext.fromCanvas(HTMLCanvasElement canvas, {bool? alpha}) {
    throw UnimplementedError();
  }
}

With Dart SDK version: 2.17.6, webdev serve crashes:

dartdevc -k arguments: --dart-sdk-summary=/opt/homebrew/Caskroom/flutter/3.0.1/flutter/bin/cache/dart-sdk/lib/_internal/ddc_outline_sound.dill --modules=amd --no-summarize --experimental-output-compiled-kernel -o packages/twod/twod.sound.ddc.js --source-map --sound-null-safety (... some names removed ...)

dart --version: 2.17.6 (stable) (Tue Jul 12 12:54:37 2022 +0200) on "macos_arm64"

Invalid argument(s): Missing canonical name for Reference to WebGLRenderingContext.fromCanvas|staticInteropFactoryStub
#0      BinaryPrinter.writeNonNullReference (package:kernel/binary/ast_to_binary.dart:941:9)
#1      BinaryPrinter.visitStaticInvocation (package:kernel/binary/ast_to_binary.dart:1733:5)
#2      StaticInvocation.accept (package:kernel/ast.dart:6289:44)
#3      BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:430:10)
#4      BinaryPrinter.writeOptionalNode (package:kernel/binary/ast_to_binary.dart:522:7)
#5      BinaryPrinter.writeVariableDeclaration (package:kernel/binary/ast_to_binary.dart:2266:5)
#6      BinaryPrinter.visitVariableDeclaration (package:kernel/binary/ast_to_binary.dart:2252:5)
#7      VariableDeclaration.accept (package:kernel/ast.dart:10431:43)
#8      BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:430:10)
#9      BinaryPrinter.writeNodeList (package:kernel/binary/ast_to_binary.dart:340:7)
#10     BinaryPrinter.visitBlock (package:kernel/binary/ast_to_binary.dart:2063:5)
#11     Block.accept (package:kernel/ast.dart:8885:43)
#12     BinaryPrinter.writeNode (package:kernel/binary/ast_to_binary.dart:430:10)
#13     BinaryPrinter.writeOptionalNode (package:kernel/binary/ast_to_binary.dart:522:7)
#14     BinaryPrinter.visitFunctionNode (package:kernel/binary/ast_to_binary.dart:1481:5)
#15     FunctionNode.accept (package:kernel/ast.dart:3846:38)
#16     BinaryPrinter.writeFunctionNode (package:kernel/binary/ast_to_binary.dart:437:10)
#17     BinaryPrinter.visitProcedure (package:kernel/binary/ast_to_binary.dart:1312:5)
#18     Procedure.accept (package:kernel/ast.dart:3151:40)
#19     BinaryPrinter.writeProcedureNode (package:kernel/binary/ast_to_binary.dart:458:10)
#20     BinaryPrinter.writeProcedureNodeList (package:kernel/binary/ast_to_binary.dart:349:7)
#21     BinaryPrinter.visitClass (package:kernel/binary/ast_to_binary.dart:1216:5)
#22     Class.accept (package:kernel/ast.dart:1408:38)
#23     BinaryPrinter.writeClassNode (package:kernel/binary/ast_to_binary.dart:472:10)
#24     BinaryPrinter.writeClassNodeList (package:kernel/binary/ast_to_binary.dart:367:7)
#25     BinaryPrinter.visitLibrary (package:kernel/binary/ast_to_binary.dart:1058:5)
#26     Library.accept (package:kernel/ast.dart:565:38)
#27     BinaryPrinter.writeLibraryNode (package:kernel/binary/ast_to_binary.dart:451:10)
#28     BinaryPrinter.writeLibraries (package:kernel/binary/ast_to_binary.dart:756:9)
#29     BinaryPrinter.writeComponentFile.<anonymous closure> (package:kernel/binary/ast_to_binary.dart:595:7)
#30     Timeline.timeSync (dart:developer/timeline.dart:157:22)
#31     BinaryPrinter.writeComponentFile (package:kernel/binary/ast_to_binary.dart:577:14)
#32     _compile (package:dev_compiler/src/kernel/command.dart:409:32)
<asynchronous suspension>
#33     compile (package:dev_compiler/src/kernel/command.dart:50:12)
<asynchronous suspension>
#34     _CompilerWorker.performRequest (package:dev_compiler/ddc.dart:68:18)
<asynchronous suspension>
#35     AsyncWorkerLoop.run (package:bazel_worker/src/worker/async_worker_loop.dart:35:20)
<asynchronous suspension>
#36     internalMain (package:dev_compiler/ddc.dart:31:5)
<asynchronous suspension>

I changed it from a factory constructor to a static method and it compiles/works as expected.

rileyporter commented 2 years ago

@nshahan, this seems like a similar issue to the one filed in #49693, but the Missing canonical name error is slightly different.

srujzs commented 2 years ago

Is it possible to try a later dev version? There were a couple changes that have landed since then (I think 2.19.0-52.0.dev was the last of them). You'll still come across factory issues, but it'll be a different error so we can de-dup. :)

FYI - I hope to tackle the factory issue soon (and static methods, which I'm surprised worked for you).

matanlurey commented 2 years ago

Is it possible to try a later dev version? There were a couple changes that have landed since then (I think 2.19.0-52.0.dev was the last of them). You'll still come across factory issues, but it'll be a different error so we can de-dup. :)

It's not trivial for me to change versions right now in the middle of a project, but I can upload what I have so far shortly.

FYI - I hope to tackle the factory issue soon (and static methods, which I'm surprised worked for you).

Just to confirm - static methods allow compilation, but didn't work. I ended up doing:

/// Methods provided to [WebGLRenderingContext] using [staticInterop].
///
/// See https://pub.dev/packages/js#interop-with-native-types-using-staticinterop.
extension WebGLRenderingContext$ on WebGLRenderingContext {
  /// Creates a [WebGLRenderingContext] via [HTMLCanvasElement$.getContext].
  static WebGLRenderingContext fromCanvas(
    HTMLCanvasElement canvas, {
    bool? alpha,
    bool? antialias,
  }) {
    final context = canvas.getContext(
      'webgl',
      _WebGLGetContextAttributes(alpha: alpha, antialias: antialias),
    );
    if (context == null) {
      throw UnsupportedError('WebGL not supported');
    }
    return context as WebGLRenderingContext;
  }
}
srujzs commented 2 years ago

Okay, I figured static methods wouldn't work, so that's consistent. Yeah, that's a fine alternative (although migration to views in the future will be a teeny bit annoying). I believe @joshualitt has used top-level methods as well to work around these factories.

matanlurey commented 2 years ago

Top-level methods work fine but then I'm expanding the top-level API surface instead of stuff living "naturally" :)

Is there plans for non-external factory constructors and static methods to be supported, either now or post views?

(At minimum I'd hope this was a static error, not a crash)

srujzs commented 2 years ago

Is there plans for non-external factory constructors and static methods to be supported, either now or post views?

Yes, definitely. These work as expected in dart2js, and it's one of my goals for Q3-ish to add support for this in @staticInterop for DDC. With views, we should get both for free by virtue of it being a language feature.

srujzs commented 1 year ago

This was added late last year, but I never closed this issue. You should be able to use non-external factory and static methods on @staticInterop types for all web compilers. You can also write these members using extension types.