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.25k stars 1.58k forks source link

Using ui-as-code features gives an error directing you at a non-existent sdk version #37898

Closed jakemac53 closed 5 years ago

jakemac53 commented 5 years ago

I attempted to use a ui as code feature and got a message from the analyzer telling me to update my min sdk constraint to 2.2.2 (:tada:).

So I did that, and updated travis to use that version, only to discover it doesn't exist. The actual required version is 2.3.0.

I don't know how much this really matters (technically the >=2.2.2 constraint will "work", since 2.3.0 is the first valid version fitting that constraint).

srawlins commented 5 years ago

I have a CL out for this, but its failing on some Flutter bots, because either Flutter or some mock Flutter packages are using 2.2.2 as a constraint, while using ui-as-code. https://ci.chromium.org/p/dart/builders/try/flutter-analyze-try/2490

bwilkerson commented 5 years ago

@stereotype441 Is this a bug?

stereotype441 commented 5 years ago

Just spoke with @bwilkerson about this. He wanted to make sure that it wasn't 2.2.2 for some intentional reason. It's not. What happened was that at the time we implemented the hint, we thought that we would release UI-as-code in Dart version 2.2.2. Later, we decided that we would release UI-as-code in Dart version 2.3, and there would never be a version 2.2.2. At this point we were in a cherry-pick season and didn't want to make unnecessary changes, so we decided to leave the warning at 2.2.2 because there was no harm in doing so; the important invariant is that versions greater than or equal to 2.2.2 have the feature, and versions less than 2.2.2 don't have it.

Now that we're not in a cherry-pick season, it seems like a no-brainer to just change the warning over to 2.3. But there's a problem with that: undoubtedly in the time since Dart 2.3 was released, someone has published a package that uses UI-as-code, and set its minimum SDK constraint to 2.2.2 (on the advice of the hint). If we change the hint to require Dart 2.3 now, then the next release of Dart will declare their package to be broken. When in reality it is fine, since there is effectively no difference between Dart 2.2.2 and Dart 2.3.

So we need to either (a) leave the code as is, and put up with the slight user confusion of having a hint that refers to a non-existent version, or (b) keep the hint logic so that it compares against version 2.2.2 when deciding whether to issue the hint, but change the hint text so that it suggests using a minimum SDK constraint of Dart 2.3. I don't have a strong preference between those two options.

@srawlins does this seem reasonable to you?

srawlins commented 5 years ago

I don't think that a third option, (c), change the logic to use 2.3.0, is out of the question. We change Hints and add Hints all the time; it doesn't alter any runtime behavior of a package that is shipped with a lower bound of 2.2.2. They just have a new action item the next time they run dartanalyzer.

That being said, I have no skin in this game, and am happy with option (b), changing the text.

srawlins commented 5 years ago

OK just adjusted my CL to fit solution (b). https://dart-review.googlesource.com/c/sdk/+/113743