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

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

Open
mit-mit opened this issue Oct 15, 2021 · 11 comments
Open

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

mit-mit opened this issue Oct 15, 2021 · 11 comments
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.

Comments

@mit-mit
Copy link
Member

mit-mit commented Oct 15, 2021

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
Copy link
Member Author

mit-mit commented Oct 15, 2021

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

@a-siva a-siva added the area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. label Oct 15, 2021
@jonasfj
Copy link
Member

jonasfj commented Oct 18, 2021

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'

@mit-mit
Copy link
Member Author

mit-mit commented Jan 4, 2022

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
Copy link
Member

jonasfj commented Jan 5, 2022

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
Copy link
Member Author

mit-mit commented Jan 21, 2022

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
Copy link
Member Author

mit-mit commented Aug 15, 2022

Paging @jonasfj

@jonasfj
Copy link
Member

jonasfj commented Aug 15, 2022

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
Copy link
Member

jonasfj commented Aug 16, 2022

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

@mit-mit
Copy link
Member Author

mit-mit commented Aug 16, 2022

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

This comment was marked as off-topic.

@mraleph

This comment was marked as outdated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool.
Projects
None yet
Development

No branches or pull requests

5 participants