feat: add suggestions to module not found error messages for file urls #21498
Conversation
dsherret
changed the title
|
are there any performance impacts? startup time? |
|
This code is only executed on error. |
There was a problem hiding this comment.
LGTM.
I was wondering if we should print out a warning when --unstable-sllopy-imports is passed to inform users that it will be slower without it and they should consider removing this flag if no other warning is printed. But we can certainly do that in another PR.
| fn test_sloppy_import_resolution_to_message() { | ||
| // none | ||
| let url = ModuleSpecifier::parse("file:https:///dir/index.js").unwrap(); | ||
| assert_eq!( | ||
| sloppy_import_resolution_to_suggestion_message( | ||
| &SloppyImportsResolution::None(&url) | ||
| ), | ||
| None, | ||
| ); | ||
| // directory | ||
| assert_eq!( | ||
| sloppy_import_resolution_to_suggestion_message( | ||
| &SloppyImportsResolution::Directory( | ||
| ModuleSpecifier::parse("file:https:///dir/index.js").unwrap() | ||
| ) | ||
| ) | ||
| .unwrap(), | ||
| "Maybe specify path to 'index.js' file in directory instead" | ||
| ); | ||
| // no ext | ||
| assert_eq!( | ||
| sloppy_import_resolution_to_suggestion_message( | ||
| &SloppyImportsResolution::NoExtension( | ||
| ModuleSpecifier::parse("file:https:///dir/index.mjs").unwrap() | ||
| ) | ||
| ) | ||
| .unwrap(), | ||
| "Maybe add a '.mjs' extension" | ||
| ); | ||
| // js to ts | ||
| assert_eq!( | ||
| sloppy_import_resolution_to_suggestion_message( | ||
| &SloppyImportsResolution::JsToTs( | ||
| ModuleSpecifier::parse("file:https:///dir/index.mts").unwrap() | ||
| ) | ||
| ) | ||
| .unwrap(), | ||
| "Maybe change the extension to '.mts'" | ||
| ); | ||
| } |
| code: Some(lsp::NumberOrString::String("no-local".to_string())), | ||
| source: Some("deno".to_string()), | ||
| message: format!( | ||
| "Unable to load a local module: {}\nMaybe add a '.ts' extension.", | ||
| temp_dir.join("a").uri_file(), | ||
| ), | ||
| ..Default::default() |
There was a problem hiding this comment.
Oh, this is super nice 👍 will it still provide a quick-fix?
There was a problem hiding this comment.
Not at the moment. I will do that in a follow-up sometime.
I was thinking about that, but it already prints a warning for every import. |
|
Right, but what if you fix all these errors and still run with the flag. Won't that cause unnecessary computation? |
Yeah, good point. I'll do another PR that adds this soon. |
For example: