-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Auto importing dart:html #41641
Comments
I think we'll want to do one of either:
cc @pq @bwilkerson @jwren |
When the code completion engine decides whether to suggest names defined in other not-yet-imported libraries, it can use the declared package dependencies to do so. We don't have quite the same information available for core libraries, but maybe there's a mapping we can make from the SDK (Dart vs. Flutter) to control the availability of them. |
Same here: I just always end up deleting I also now notice that it also adds other imports by itself without me knowing about it.
But (content deleted for brevity)
Logs
|
Same issue here, along with my 3 friends facing this issue. |
Thanks for the feedback (and sorry for the continued hassle). Tracking on the SDK here: #41641. |
This came up in Dart-Code/Dart-Code#3265 (comment) too, and Eric made an interesting comment about non-Flutter code. It reminded me that I've accidentally imported dart:html (quite a lot, actually) into a CLI project I have (a static website generator - which has a lot of overlap with class names that are also in So it may be advantageous for the solution to not be Flutter-specific/based on the SDK type, but something that can be opted-in to pure CLI projects too. (I just found there's a lint |
@bwilkerson the comment about |
It isn't necessarily crazy. But as Danny pointed out (on Apr 8, 2021), a non-Flutter-specific solution might be better (if we can figure out when these libraries should not be suggested). Aside from that, the only concern I have is performance. Currently the lint parses the Would we want this to be transitive? That is, not suggest items in libraries that directly or indirectly import one of these libraries? |
I don't understand this idea. Wouldn't this be based strictly on whether VM-only libraries are imported (like
Why would have to execute the lint rule? Don't we know (without reparsing) whether the lint rule is enabled or not?
I don't think so. The main pain point as I understand it is accidentally importing things from |
I don't think so. The question isn't about whether we're importing non-web libraries, the question is whether it's valid to import web-only libraries. Or, more generally, if we have platform-specific libraries in the SDK, then we shouldn't be suggesting them if the code isn't targeting the required platform.
Yes, but I wasn't talking about the lint rule. The question that code completion would need to answer is: is this a Flutter app? The way the lint rule answers this question is to parse the
Right. But if we shouldn't suggest names from |
Ah I see. Very curious.
I think I got lost in asking my question. What do you mean by "when there's an import of |
I was riffing on your question of "Wouldn't this be based strictly on whether VM-only libraries are imported", which changed the discussion from "is this a Flutter app" to "is this library importing a VM-only library". I should have been more clear. So let me restate my previous assertion: I think the question is "is the library in which completion is being performed expected to only run on platforms that allow the use of the library under consideration", not "is there an import that makes it wrong to also import this library". It might be easier to answer that question if we only answer it for Flutter apps (because I think the supported platforms are better specified there), but it's a question I'd like us to eventually be able to answer for any code. |
I agree.
I'm not sure it would make it easier, as dart:html is a valid import for an app running Flutter on the web.
I agree. But there is no mechanism for this today, right? Are we circling into an answer that we cannot implement this feature today? |
Probably. Sigh. |
Haha yeah. I think there is a whole collection of problems that need the answer, "is this expected to be compiled for web? is this expected to be compiled for CLI?" In google3 we have the "platforms" list which solves this. Actually, come to think of it, I'm not sure where the pub badges come from. Like the file package does not have 'web' listed, but the markdown package seems safe for everything, and the camera "package" only lists flutter. Definitely might be worth a look. Definitely maybe. |
This came up at Dart-Code/Dart-Code#4693 with
Could we have something similar in analysis_options? (I saw analysis_options and not pubspec, because I can imagine having CLI scripts in |
Of course, I'd really prefer to have some existing mechanism be able to answer this question (an existing field in the pubspec? Some existing Flutter configuration? The presence of a file? Barring that, we could add support in analysis_options.yaml, or maybe do something clever. Could we track whether |
Presence of dart:html in some files might mean it's safe to suggest from dart:html, but I don't know if the opposite is true (or at least, ideal). Conditional imports might also confuse things further... DevTools imports |
Yeah conditional imports would have to be taken into account, and as you mentioned earlier, maybe you have a web app, but use dart:io in the tool/ directory, etc. I don't think it could be made foolproof. |
At this point I am comfortable completely disabling auto-importing dart:html as while the library is not deprecated yet, it doesn't work with WASM and will be deprecated at some point. |
Done by @scheglov in https://dart-review.googlesource.com/c/sdk/+/348081 (as per #54610) :) |
Thanks @scheglov! |
Flutter keeps auto importing dart:html when I try to use a Text widget. I am using VsCode.
Flutter Doctor:
The text was updated successfully, but these errors were encountered: