sdk icon indicating copy to clipboard operation
sdk copied to clipboard

Poor warnings when adding a Flutter plugin to a Dart app

Open mit-mit opened this issue 4 years ago • 11 comments

Repro steps:

  1. dart create repro
  2. add url_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<String> arguments) {
  launch('https://drt.dev');
}
  1. dart analyze

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

  1. 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 avatar Oct 15 '21 12:10 mit-mit

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

mit-mit avatar Oct 15 '21 12:10 mit-mit

This is possible when:

  • using the Dart SDK installed along with the Flutter SDK, because:
    • Flutter wraps the dart binary in a bat/bash script that defines FLUTTER_ROOT, and,
    • dart pub detects Flutter SDK location based on relative path (context: pub#3045)
  • the user has FLUTTER_ROOT defined.

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'

jonasfj avatar Oct 18 '21 11:10 jonasfj

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

mit-mit avatar Jan 04 '22 13:01 mit-mit

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.

jonasfj avatar Jan 05 '22 11:01 jonasfj

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 avatar Jan 21 '22 08:01 mit-mit

Paging @jonasfj

mit-mit avatar Aug 15 '22 11:08 mit-mit

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 avatar Aug 15 '22 14:08 jonasfj

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

jonasfj avatar Aug 16 '22 09:08 jonasfj

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

Yes, perhaps fixing the messaging is enough.

mit-mit avatar Aug 16 '22 09:08 mit-mit

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
  1. add the following to main.dart:
import 'package:provider/provider.dart';
void main() {
  Provider.value(value: 0);
}
  1. 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"

osaxma avatar Sep 18 '22 09:09 osaxma

@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.

mraleph avatar Sep 19 '22 07:09 mraleph