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.08k stars 1.56k forks source link

Poor warnings when adding a Flutter plugin to a Dart app #47470

Open mit-mit opened 2 years ago

mit-mit commented 2 years ago

Repro steps:

  1. dart create repro
  2. addurl_launcher:underdependencies:`
  3. dart pub get

=> No warning. Should we be saying something like Warning: Package url_launcher requires the Flutter SDK?

  1. Edit bin/repro.dart to contain:
    
    import 'package:url_launcher/url_launcher.dart';

void main(List arguments) { launch('https://drt.dev'); }

5. `dart analyze`

=> No warning. Should we be saying something like "Error: package:url_launcher requires the Flutter SDK"?

6. `dart run`

=> A huge amount of error output like:

: Error: Not found: 'dart:ui' ../…/foundation/basic_types.dart:9 export 'dart:ui' show VoidCallback; ^ : Error: Not found: 'dart:ui' ../…/foundation/binding.dart:8 import 'dart:ui' as ui show SingletonFlutterWindow, Brightness, PlatformDispatcher, window; ^ : Error: Not found: 'dart:ui' ../…/foundation/debug.dart:5 import 'dart:ui' as ui show Brightness;

mit-mit commented 2 years ago

Not entirely sure what the right experience is, but this feels rough, cc @jonasfj @bwilkerson @jacob314

jonasfj commented 2 years ago

This is possible when:

As I recall we did pub#3045 to solves issues for tooling, see pub#2307.


A reasonable opinion is that when we are able to locate a Flutter SDK, then dart <tool> commands should be able to work with said Flutter SDK. Hence, dart pub|test|format|analyze|fix|... shouldn't abort telling users to do flutter <tool> instead. But perhaps we need a way for Dart tooling to warn that you're missing an required SDK.

Or at-least we should improve the error message, we can do better than:

Error: Not found: 'dart:ui'

mit-mit commented 2 years ago

I don't think the resolution should depend on where the dart command comes from; I think it should depend on what is being declared in the pubspec.yaml it's resolving. And in this case, I think we should look at what SDKs are being declared a dependency on. Only pubspecs with this content should allow packages that depend on the Flutter SDK:

dependencies:
  flutter:
    sdk: flutter
jonasfj commented 2 years ago
dependencies:
  flutter:
    sdk: flutter

Is a direct-dependency on package "flutter" from "Flutter SDK". I don't think that's the correct way to express that something is a Flutter project.


If we wanted apps to explicitly specify that they require Flutter SDK, then an SDK constraint would better:

environment:
  sdk: >=2.12.0 <3.0.0
  flutter: >= 2.0.0

We already require the Dart SDK constraint to be present (expressed in environment.sdk), this requirement was introduced in Dart 2.12. But we don't require environment.flutter to be specified for packages depending on Flutter to be installed. This would be interesting to do, but it would affect a LOT of a packages and applications.

We can reasonably make it a requirement for the root pubspec.yaml, then it only affects the users current project. That's also what we did for Dart SDK constraint. But this would affect a LOT of flutter projects. As of writing the default application template for Flutter doesn't specify environment.flutter.

If we want to do this we'll need some buy-in from Flutter as this would affect a lot of Flutter users.

mit-mit commented 2 years ago

Is a direct-dependency on package "flutter" from "Flutter SDK". I don't think that's the correct way to express that something is a Flutter project.

Why? It literally says "I depend on the Flutter SDK"!

mit-mit commented 2 years ago

Paging @jonasfj

jonasfj commented 2 years ago

Why? It literally says "I depend on the Flutter SDK"!

It says I depend on "package:flutter" from the Flutter SDK. I guess when writing Flutter a flutter app or package that is common though.

I'm not sure what the right thing to do here is. Do we really want pubspec.yaml for Flutter apps/packages to be special inorder for them to be able to depend on another Flutter package. It seems counter intuitive to me. Like the right thing to do, is to have dart pub get complain that you should be running flutter pub get because some dependency has a dependency on the Flutter SDK. On the flip side, I want dart pub get to work for Flutter projects in the future. Maybe we haven't fully worked out a consistent concept for the relationships between packages, SDKs and tools.

jonasfj commented 2 years ago

Maybe we should special case the error message: : Error: Not found: 'dart:ui'?

mit-mit commented 2 years ago

Yes, I hear you on the fact that there is a difference between SDK constraints and dependencies...

Yes, perhaps fixing the messaging is enough.

osaxma commented 1 year ago

I faced a similar issue today after adding a flutter package which I thought it was dart compatible.

The error message was not very clear and it didn't mention anything about the package or flutter:

Crash when compiling null,
at character offset null:
Null check operator used on a null value
#0      InferableTypeBuilderMixin.type (package:front_end/src/fasta/builder/type_builder.dart:392:29)
#1      InferableTypeBuilder.inferType (package:front_end/src/fasta/builder/omitted_type_builder.dart:155:12)
#2      SourceLoader.performTopLevelInference (package:front_end/src/fasta/source/source_loader.dart:2358:19)
#3      KernelTarget.buildOutlines.<anonymous closure> (package:front_end/src/fasta/kernel/kernel_target.dart:532:14)
<asynchronous suspension>
#4      withCrashReporting (package:front_end/src/fasta/crash.dart:122:12)
<asynchronous suspension>
#5      _buildInternal (package:front_end/src/kernel_generator_impl.dart:139:7)
<asynchronous suspension>
#6      withCrashReporting (package:front_end/src/fasta/crash.dart:122:12)
<asynchronous suspension>
#7      generateKernel.<anonymous closure> (package:front_end/src/kernel_generator_impl.dart:47:12)
<asynchronous suspension>
#8      generateKernel (package:front_end/src/kernel_generator_impl.dart:46:10)
<asynchronous suspension>
#9      kernelForModule (package:front_end/src/api_prototype/kernel_generator.dart:100:11)
<asynchronous suspension>
#10     SingleShotCompilerWrapper.compileInternal (file:///opt/s/w/ir/x/w/sdk/pkg/vm/bin/kernel_service.dart:412:11)
<asynchronous suspension>
#11     Compiler.compile.<anonymous closure> (file:///opt/s/w/ir/x/w/sdk/pkg/vm/bin/kernel_service.dart:221:45)
<asynchronous suspension>
#12     _processLoadRequest (file:///opt/s/w/ir/x/w/sdk/pkg/vm/bin/kernel_service.dart:914:37)
<asynchronous suspension>

It would've been useful if a warning/error was present in pubspec.yaml or when running dart pub get.

To Reproduce:

  1. create a project and add a flutter package (provider as an example):
    dart create repro
    cd repro
    dart pub add provider
  2. add the following to main.dart:
    import 'package:provider/provider.dart';
    void main() {
    Provider.value(value: 0);
    }
  3. run dart run bin/main.dart

I'm using Dart SDK version: 2.18.0 (stable) (Fri Aug 26 10:22:54 2022 +0000) on "macos_x64"

mraleph commented 1 year ago

@osaxma the crash you are seeing is tracked in https://github.com/dart-lang/sdk/issues/49641, it's unrelated to the topic of this issue.