Repository navigation
The widget build does not provide return type #3975
Description
Describe the bug
There is no return type after call of snippets for stateful and stateless widget.
Expected behavior
There should be a return type.
Screenshots
Please complete the following information:
- Operating System and version: Mac OS version:12.3.1
- VS Code version: 1.67.2
- Dart extension version:v3.40.0
- Dart/Flutter SDK version:3.0.0
- Target device (if the issue relates to Flutter debugging):
Activity
DanTup commentedon May 20, 2022
@saajanpandey I'm not sure I understand the problem. Can you explain exactly what is missing, perhaps by modifying the code in the way you'd expect it to appear, for comparison?
saajanpandey commentedon May 20, 2022
@DanTup in widget build function there should be a return such as shown in image above. In the Dart extension version:v3.40.0 there was no return shown.
[analysis_server] Include placeholder Container() in Flutter snippet …
DanTup commentedon May 26, 2022
Thanks for clarifying. I have made some changes to the snippets when provided by the server that will include
return Container();again, and will selectContainer()as the final tab stop, so you can easily type over it.dart-lang/sdk@cd4d9c6
dart-lang/sdk@fa8a33f
The change is in the Dart analysis server (which now provides the snippets for VS Code), so it will require a future Dart/Flutter SDK release to show up, it won't show up immediately in the Dart-Code extension release.
Thanks!
gspencergoog commentedon Sep 1, 2022
I wanted to point out that we have a lint turned on in the Flutter repo (sized_box_for_whitespace) which recommends using a
const SizedBox()as opposed toContainer()in these situations because containers are less efficient at being empty space (partly because they can't be const).So, for anyone with that lint enabled, this will insert a line that fails the lint.
DanTup commentedon Sep 1, 2022
@gspencergoog do you have any feeling for how often a user would want a container (eg. adding some arguments) versus replacing whatever is given with other widgets?
It's easy enough to change, but if the most common thing is for additional arguments to be added and the
Containerleft, that might mean the user has to make more changes. I haven't done enough Flutter to have any feeling for what's common so you probably have a better idea of what makes most sense.If we do change here, we should also do the same in IntelliJ to keep them in sync (FYI @stevemessick).
gspencergoog commentedon Sep 1, 2022
I think the most common case would be replacing it with another widget. Containers are sometimes used, but there are so many other possibilities that it's unlikely to be the "common case". You could also just go with
const Placeholder(), which is a valid widget, and the name says what it is (it draws a box with an X across the middle).PollyGlot commentedon Sep 1, 2022
@gspencergoog Not many devs know the difference between
Container()andSizedBox(), I mean from a ground perspective and the most don't care about it. I think we are all used to useContainer()as a "placeholder" or an empty return widget. So I don't see any problem if a snippet returns aContainer(). Anyway it will be changed for another widget while working. And if it is not, there is always a linter that will notify to change it to SizedBox / ColoredBox, etc.gspencergoog commentedon Sep 1, 2022
I'm not worried that it'll be misinterpreted,
Containeris a fine default, it just seems like we shouldn't insert code that violates a lint, even if that lint isn't turned on everywhere.gspencergoog commentedon Sep 1, 2022
Also, if
const SizedBox()is the default, maybe more devs will look into it and wonder why we choseconst SizedBox()overContainer()and learn something (because if you leave it in, it's less efficient to use aContainer).I feel like
const Placeholder()would be my preferred choice, simply because it's not likely to stay, and because it says what it is. But I'd be fine with anything that doesn't trigger a lint.DanTup commentedon Sep 1, 2022
I like this most too, it seems clear what it is in code, and if it's rendering something that Flutter users know as a placeholder in the app too, that also seems better than rendering blank space.
I've filed issues here and in flutter-intellij to change these:
bwilkerson commentedon Nov 7, 2022
I'm fine with changing the snippets to use
Placeholder; it seems like a good default.But in the interest of spreading information I'll point out that our snippets, because they're implemented in the language server, have the ability to determine which lints are enabled and to tailor their behavior accordingly. For example, there are a pair of lints related to whether to use a single quote of a double quote for a string literal. If we had a snippet that produced a string literal it could use the user's preferred style. Similar logic can be applied in other situations and for other lints.
(The support for using lint enablement as a hint to inform code generation is relatively new and not used everywhere it should be. Please let us know if there are situations you know of where we should prioritize improving the generated code.)
[analysis_server] Change Flutter snippets to use Placeholder