Dart-code: Import quick-fixes do not provide relative paths
Before this version the auto import worked with relative paths, sometimes in the code actions I could find both the command for import with relative path and absolute path. Now there's only the possibility to import with absolute path, and this is tragic considering the problems dart has using both of them in the same project ( I worked with relative paths until now ).
darioielardi
All 19 comments
Have you upgraded Dart or Flutter recently? The quick-fixes come from the Dart SDK so it seems more likely that if this has changed it was triggered by that.
@bwilkerson @scheglov Do you know if anything has changed around this recently? I can repro this, but don't know if it's changed or just that the options seem to vary (I recall seeing that sometimes I'd only get one option and sometimes I'd get both).
Here, the import I deleted was a relative path, but the only suggestion is a package path.
DanTup
on 16 Jun 2018
Analysis Server prefers suggesting absolute imports.
From time to time we consider making it an option whether to use absolute or relative imports, but this has not been implemented so far.
There were two related changes recently (discover all available libraries in dependency packages, and unifying the way we write imports to DartFileEditBuilder). I cannot find code that would suggest both kinds of imports - relative and absolute, in the changes history. It seems like now we have the correct (even if not what the user wants) behavior.
scheglov
on 16 Jun 2018
Do you mean that it might have previously suggested relative imports, just not at the same time? It's possible I mis-remembered about them being at the same time, but I'm sure I've seen relative imports before (though I did just try an older SDK but still failed to get any relative imports, so now I'm quite confused!).
It seems like now we have the correct (even if not what the user wants) behavior.
Are there any published guidelines on how things should be imported? I'm confused by what the rules are - flutter_tools is full of relative imports and I'm sure using package: imports causes issues.
DanTup
on 16 Jun 2018
Found some more notes in this case:
https://github.com/dart-lang/sdk/issues/32916
Interestingly there's this comment:
now having the problem again. it only offers me relative path and no package import
So it seems that it probably did suggest relative paths in some cases in the past; so there may have been a change (though based on comments in that issue and this one; it seems like this might be design).
DanTup
on 16 Jun 2018
There is a rule now, and it is to use relative imports inside a package lib/. This is in a contradiction with one lint, which helps you avoid problems when you run something from your lib/ folder. There were some discussions about changing rules for URIs of files in lib/ folders (there used to be file:// URIs, not package:// URIs), but nothing changed yet @eernstg.
I'd say that we need a new lint that insists that you should use relative imports, and Analysis Server should use relative imports. Running from lib/ folder is weird anyway.
And sometimes there is no choice - imports in test/ have to be relative when you import from the same test/.
scheglov
on 17 Jun 2018
I'd say that we need a new lint that insists that you should use relative imports, and Analysis Server should use relative imports.
This seems reasonable to me. Do we have SDK issues for these or should we open some?
DanTup
on 18 Jun 2018
An issue for mixing absolute and relative import URIs: https://github.com/dart-lang/sdk/issues/33076.
Unfortunately Flutter does use lib/main.dart, and people have problems with using relative URIs.
There is a language issue for fixing this: https://github.com/dart-lang/sdk/issues/32601. Not fixed yet.
So, at least for some time we will have to keep using relative imports optional, e.g. only when the corresponding lint is enabled. It seems that https://github.com/dart-lang/sdk/issues/32916 can be used as an issue to track using relative imports.
scheglov
on 18 Jun 2018
If the fix is likely some way off, is it worth doing something in the server to help (for example offering both types of imports and letting the user pick; or trying to do something clever based on other imports)?
It's a bit of a pain quick-fixing package: imports and then manually having to know how many ../'s to add in or whatever; though it might not be worth spending time on if it's going to sort itself out soon.
DanTup
on 19 Jun 2018
Yeah, that's one of the reasons I prefer package: URI's to relative paths: it's much easier to copy/paste them and to re-organize files. Personally, I'd be sad if we dropped support for users that still want to use package: URIs within the same package.
bwilkerson
on 19 Jun 2018
I think the server should support offer whatever is valid; so if(!) both are valid it should offer both and let the user pick (like they could if typing manually). If there are existing imports for the same package, then it'd also be nice to only show the matching type. Showing double the quick-fixes in some cases doesn't seem like the end of the world; it's probably not common for there to be large numbers of candidates for importing.
In an ideal world I think it would make no difference (eg. you can mix and match, and they're normalised in some way) but I don't know whether that would work everywhere or is feasible.
DanTup
on 19 Jun 2018
@DanTup could we leverage both auto import and quick fix? Like Typescript auto import, Autocomplete will show options when we are typing and after we choose the suggestion, files/library import will be added at the top of that file. We can adjust the import statement by hit ctrl+z (undo adding import) and ctrl+. to show quick fix.
sandangel
on 4 Jul 2018
Showing "unimported" items in the completion and then inserting the imports need to be implemented in the analysis server (including addressing https://github.com/dart-lang/sdk/issues/27220).
It'd be good to have (I've been using it a lot in TypeScript working on Dart Code lately, and I really like it!) but I don't know if it changes anything here - it would end up inserting the same path that is in the quick-fix list.
DanTup
on 4 Jul 2018
@DanTup I think we should take the same approach with typescript auto import, I have not had any issue with that.
sandangel
on 4 Jul 2018
@sandangel I'm not entirely sure what you mean?
DanTup
on 4 Jul 2018
@DanTup I mean when we type something, for example, first 3 characters of a widget class named LoginPage, vscode will search through files in lib/ folder and give us the LoginPage in autocompletion, then when we hit Tab, we will add the import path at the top of file. If we don't hit tab and type LoginPage, the error will show up, then we hit ctrl+. to trigger quick-fix, vscode will show up the option to include import path. That's how auto import in typescript work now and I think we should implement dart auto import work in the same way.
sandangel
on 4 Jul 2018
Yep, that's what I meant in my previous comment. To support this, we need to have the server return all items in the completion list (even when not imported) and also provide us the information for how to insert the import (which would be the same path that appears in the quick-fix; it wouldn't make sense to be different). The code completion is all supported by the server, not implemented directly in Dart Code.
There may be some serious performance implications of this though - currently the completion list is not filtered by the server (but instead by the client), so importing "everything" might make the server response orders of magnitude larger than it is currently.
I've opened https://github.com/Dart-Code/Dart-Code/issues/1060 to track that as it's different to this issue, but it will require work in the server. There are a few related cases in the SDK repo:
DanTup
on 4 Jul 2018
@DanTup I don't see typescript auto import has this performance issue although the number of files to scan in node_modules is much larger than dart package I think. I'm curious how did typescript team solve this problem.
sandangel
on 4 Jul 2018
Let's move discussion about the import idea to https://github.com/Dart-Code/Dart-Code/issues/1060 to keep this issue focused on the relative/package paths as I don't think they're tied together. I replied to your comment here.
DanTup
on 4 Jul 2018
Cleaning up the issue tracker a little. Closing this in favour of the SDK issue https://github.com/dart-lang/sdk/issues/32916 since I don't think there's anything VS code-specific here.
DanTup
on 25 Oct 2018
Related issues
DanTup
路
3Comments
kinex
路
3Comments
ifredom
路
3Comments
rajeshjeshar
路
4Comments
Matt-Gleich
路
4Comments
Most helpful comment
Yep, that's what I meant in my previous comment. To support this, we need to have the server return all items in the completion list (even when not imported) and also provide us the information for how to insert the import (which would be the same path that appears in the quick-fix; it wouldn't make sense to be different). The code completion is all supported by the server, not implemented directly in Dart Code.
There may be some serious performance implications of this though - currently the completion list is not filtered by the server (but instead by the client), so importing "everything" might make the server response orders of magnitude larger than it is currently.
I've opened https://github.com/Dart-Code/Dart-Code/issues/1060 to track that as it's different to this issue, but it will require work in the server. There are a few related cases in the SDK repo: