Skip to content

Commit 89c92b8

Browse files
authored
fix(check): move module not found errors to typescript diagnostics (denoland#27533)
Instead of hard erroring, we now surface module not found errors as TypeScript diagnostics (we have yet to show the source code of the error, but something we can improve over time).
1 parent 18b813b commit 89c92b8

File tree

37 files changed

+663
-337
lines changed

37 files changed

+663
-337
lines changed

Cargo.lock

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ deno_core = { version = "0.327.0" }
5353
deno_bench_util = { version = "0.178.0", path = "./bench_util" }
5454
deno_config = { version = "=0.42.0", features = ["workspace", "sync"] }
5555
deno_lockfile = "=0.24.0"
56-
deno_media_type = { version = "0.2.0", features = ["module_specifier"] }
56+
deno_media_type = { version = "0.2.3", features = ["module_specifier"] }
5757
deno_npm = "=0.27.0"
5858
deno_path_util = "=0.3.0"
5959
deno_permissions = { version = "0.43.0", path = "./runtime/permissions" }

cli/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ deno_config.workspace = true
7474
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
7575
deno_doc = { version = "=0.161.3", features = ["rust", "comrak"] }
7676
deno_error.workspace = true
77-
deno_graph = { version = "=0.86.6" }
77+
deno_graph = { version = "=0.86.7" }
7878
deno_lint = { version = "=0.68.2", features = ["docs"] }
7979
deno_lockfile.workspace = true
8080
deno_npm.workspace = true

cli/errors.rs

Lines changed: 44 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -26,51 +26,55 @@ fn get_diagnostic_class(_: &ParseDiagnostic) -> &'static str {
2626
"SyntaxError"
2727
}
2828

29-
fn get_module_graph_error_class(err: &ModuleGraphError) -> &'static str {
30-
use deno_graph::JsrLoadError;
31-
use deno_graph::NpmLoadError;
32-
29+
pub fn get_module_graph_error_class(err: &ModuleGraphError) -> &'static str {
3330
match err {
3431
ModuleGraphError::ResolutionError(err)
3532
| ModuleGraphError::TypesResolutionError(err) => {
3633
get_resolution_error_class(err)
3734
}
38-
ModuleGraphError::ModuleError(err) => match err {
39-
ModuleError::InvalidTypeAssertion { .. } => "SyntaxError",
40-
ModuleError::ParseErr(_, diagnostic) => get_diagnostic_class(diagnostic),
41-
ModuleError::WasmParseErr(..) => "SyntaxError",
42-
ModuleError::UnsupportedMediaType { .. }
43-
| ModuleError::UnsupportedImportAttributeType { .. } => "TypeError",
44-
ModuleError::Missing(_, _) | ModuleError::MissingDynamic(_, _) => {
45-
"NotFound"
46-
}
47-
ModuleError::LoadingErr(_, _, err) => match err {
48-
ModuleLoadError::Loader(err) => get_error_class_name(err.as_ref()),
49-
ModuleLoadError::HttpsChecksumIntegrity(_)
50-
| ModuleLoadError::TooManyRedirects => "Error",
51-
ModuleLoadError::NodeUnknownBuiltinModule(_) => "NotFound",
52-
ModuleLoadError::Decode(_) => "TypeError",
53-
ModuleLoadError::Npm(err) => match err {
54-
NpmLoadError::NotSupportedEnvironment
55-
| NpmLoadError::PackageReqResolution(_)
56-
| NpmLoadError::RegistryInfo(_) => "Error",
57-
NpmLoadError::PackageReqReferenceParse(_) => "TypeError",
58-
},
59-
ModuleLoadError::Jsr(err) => match err {
60-
JsrLoadError::UnsupportedManifestChecksum
61-
| JsrLoadError::PackageFormat(_) => "TypeError",
62-
JsrLoadError::ContentLoadExternalSpecifier
63-
| JsrLoadError::ContentLoad(_)
64-
| JsrLoadError::ContentChecksumIntegrity(_)
65-
| JsrLoadError::PackageManifestLoad(_, _)
66-
| JsrLoadError::PackageVersionManifestChecksumIntegrity(..)
67-
| JsrLoadError::PackageVersionManifestLoad(_, _)
68-
| JsrLoadError::RedirectInPackage(_) => "Error",
69-
JsrLoadError::PackageNotFound(_)
70-
| JsrLoadError::PackageReqNotFound(_)
71-
| JsrLoadError::PackageVersionNotFound(_)
72-
| JsrLoadError::UnknownExport { .. } => "NotFound",
73-
},
35+
ModuleGraphError::ModuleError(err) => get_module_error_class(err),
36+
}
37+
}
38+
39+
pub fn get_module_error_class(err: &ModuleError) -> &'static str {
40+
use deno_graph::JsrLoadError;
41+
use deno_graph::NpmLoadError;
42+
43+
match err {
44+
ModuleError::InvalidTypeAssertion { .. } => "SyntaxError",
45+
ModuleError::ParseErr(_, diagnostic) => get_diagnostic_class(diagnostic),
46+
ModuleError::WasmParseErr(..) => "SyntaxError",
47+
ModuleError::UnsupportedMediaType { .. }
48+
| ModuleError::UnsupportedImportAttributeType { .. } => "TypeError",
49+
ModuleError::Missing(_, _) | ModuleError::MissingDynamic(_, _) => {
50+
"NotFound"
51+
}
52+
ModuleError::LoadingErr(_, _, err) => match err {
53+
ModuleLoadError::Loader(err) => get_error_class_name(err.as_ref()),
54+
ModuleLoadError::HttpsChecksumIntegrity(_)
55+
| ModuleLoadError::TooManyRedirects => "Error",
56+
ModuleLoadError::NodeUnknownBuiltinModule(_) => "NotFound",
57+
ModuleLoadError::Decode(_) => "TypeError",
58+
ModuleLoadError::Npm(err) => match err {
59+
NpmLoadError::NotSupportedEnvironment
60+
| NpmLoadError::PackageReqResolution(_)
61+
| NpmLoadError::RegistryInfo(_) => "Error",
62+
NpmLoadError::PackageReqReferenceParse(_) => "TypeError",
63+
},
64+
ModuleLoadError::Jsr(err) => match err {
65+
JsrLoadError::UnsupportedManifestChecksum
66+
| JsrLoadError::PackageFormat(_) => "TypeError",
67+
JsrLoadError::ContentLoadExternalSpecifier
68+
| JsrLoadError::ContentLoad(_)
69+
| JsrLoadError::ContentChecksumIntegrity(_)
70+
| JsrLoadError::PackageManifestLoad(_, _)
71+
| JsrLoadError::PackageVersionManifestChecksumIntegrity(..)
72+
| JsrLoadError::PackageVersionManifestLoad(_, _)
73+
| JsrLoadError::RedirectInPackage(_) => "Error",
74+
JsrLoadError::PackageNotFound(_)
75+
| JsrLoadError::PackageReqNotFound(_)
76+
| JsrLoadError::PackageVersionNotFound(_)
77+
| JsrLoadError::UnknownExport { .. } => "NotFound",
7478
},
7579
},
7680
}

cli/factory.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -753,6 +753,7 @@ impl CliFactory {
753753
self.module_graph_builder().await?.clone(),
754754
self.node_resolver().await?.clone(),
755755
self.npm_resolver().await?.clone(),
756+
self.sys(),
756757
)))
757758
})
758759
.await

cli/graph_util.rs

Lines changed: 80 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ use crate::cache::ModuleInfoCache;
5050
use crate::cache::ParsedSourceCache;
5151
use crate::colors;
5252
use crate::errors::get_error_class_name;
53+
use crate::errors::get_module_graph_error_class;
5354
use crate::file_fetcher::CliFileFetcher;
5455
use crate::npm::CliNpmResolver;
5556
use crate::resolver::CjsTracker;
@@ -164,29 +165,15 @@ pub fn graph_walk_errors<'a>(
164165
roots.contains(error.specifier())
165166
}
166167
};
167-
let mut message = match &error {
168-
ModuleGraphError::ResolutionError(resolution_error) => {
169-
enhanced_resolution_error_message(resolution_error)
170-
}
171-
ModuleGraphError::TypesResolutionError(resolution_error) => {
172-
format!(
173-
"Failed resolving types. {}",
174-
enhanced_resolution_error_message(resolution_error)
175-
)
176-
}
177-
ModuleGraphError::ModuleError(error) => {
178-
enhanced_integrity_error_message(error)
179-
.or_else(|| enhanced_sloppy_imports_error_message(sys, error))
180-
.unwrap_or_else(|| format_deno_graph_error(error))
181-
}
182-
};
183-
184-
if let Some(range) = error.maybe_range() {
185-
if !is_root && !range.specifier.as_str().contains("/$deno$eval") {
186-
message.push_str("\n at ");
187-
message.push_str(&format_range_with_colors(range));
188-
}
189-
}
168+
let message = enhance_graph_error(
169+
sys,
170+
&error,
171+
if is_root {
172+
EnhanceGraphErrorMode::HideRange
173+
} else {
174+
EnhanceGraphErrorMode::ShowRange
175+
},
176+
);
190177

191178
if graph.graph_kind() == GraphKind::TypesOnly
192179
&& matches!(
@@ -198,10 +185,61 @@ pub fn graph_walk_errors<'a>(
198185
return None;
199186
}
200187

201-
Some(custom_error(get_error_class_name(&error.into()), message))
188+
if graph.graph_kind().include_types()
189+
&& (message.contains(RUN_WITH_SLOPPY_IMPORTS_MSG)
190+
|| matches!(
191+
error,
192+
ModuleGraphError::ModuleError(ModuleError::Missing(..))
193+
))
194+
{
195+
// ignore and let typescript surface this as a diagnostic instead
196+
log::debug!("Ignoring: {}", message);
197+
return None;
198+
}
199+
200+
Some(custom_error(get_module_graph_error_class(&error), message))
202201
})
203202
}
204203

204+
#[derive(Debug, PartialEq, Eq)]
205+
pub enum EnhanceGraphErrorMode {
206+
ShowRange,
207+
HideRange,
208+
}
209+
210+
pub fn enhance_graph_error(
211+
sys: &CliSys,
212+
error: &ModuleGraphError,
213+
mode: EnhanceGraphErrorMode,
214+
) -> String {
215+
let mut message = match &error {
216+
ModuleGraphError::ResolutionError(resolution_error) => {
217+
enhanced_resolution_error_message(resolution_error)
218+
}
219+
ModuleGraphError::TypesResolutionError(resolution_error) => {
220+
format!(
221+
"Failed resolving types. {}",
222+
enhanced_resolution_error_message(resolution_error)
223+
)
224+
}
225+
ModuleGraphError::ModuleError(error) => {
226+
enhanced_integrity_error_message(error)
227+
.or_else(|| enhanced_sloppy_imports_error_message(sys, error))
228+
.unwrap_or_else(|| format_deno_graph_error(error))
229+
}
230+
};
231+
232+
if let Some(range) = error.maybe_range() {
233+
if mode == EnhanceGraphErrorMode::ShowRange
234+
&& !range.specifier.as_str().contains("/$deno$eval")
235+
{
236+
message.push_str("\n at ");
237+
message.push_str(&format_range_with_colors(range));
238+
}
239+
}
240+
message
241+
}
242+
205243
pub fn graph_exit_integrity_errors(graph: &ModuleGraph) {
206244
for error in graph.module_errors() {
207245
exit_for_integrity_error(error);
@@ -835,18 +873,19 @@ pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
835873
message
836874
}
837875

876+
static RUN_WITH_SLOPPY_IMPORTS_MSG: &str =
877+
"or run with --unstable-sloppy-imports";
878+
838879
fn enhanced_sloppy_imports_error_message(
839880
sys: &CliSys,
840881
error: &ModuleError,
841882
) -> Option<String> {
842883
match error {
843884
ModuleError::LoadingErr(specifier, _, ModuleLoadError::Loader(_)) // ex. "Is a directory" error
844885
| ModuleError::Missing(specifier, _) => {
845-
let additional_message = CliSloppyImportsResolver::new(SloppyImportsCachedFs::new(sys.clone()))
846-
.resolve(specifier, SloppyImportsResolutionKind::Execution)?
847-
.as_suggestion_message();
886+
let additional_message = maybe_additional_sloppy_imports_message(sys, specifier)?;
848887
Some(format!(
849-
"{} {} or run with --unstable-sloppy-imports",
888+
"{} {}",
850889
error,
851890
additional_message,
852891
))
@@ -855,6 +894,19 @@ fn enhanced_sloppy_imports_error_message(
855894
}
856895
}
857896

897+
pub fn maybe_additional_sloppy_imports_message(
898+
sys: &CliSys,
899+
specifier: &ModuleSpecifier,
900+
) -> Option<String> {
901+
Some(format!(
902+
"{} {}",
903+
CliSloppyImportsResolver::new(SloppyImportsCachedFs::new(sys.clone()))
904+
.resolve(specifier, SloppyImportsResolutionKind::Execution)?
905+
.as_suggestion_message(),
906+
RUN_WITH_SLOPPY_IMPORTS_MSG
907+
))
908+
}
909+
858910
fn enhanced_integrity_error_message(err: &ModuleError) -> Option<String> {
859911
match err {
860912
ModuleError::LoadingErr(

cli/module_loader.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ use deno_graph::JsModule;
3636
use deno_graph::JsonModule;
3737
use deno_graph::Module;
3838
use deno_graph::ModuleGraph;
39+
use deno_graph::ModuleGraphError;
3940
use deno_graph::Resolution;
4041
use deno_graph::WasmModule;
4142
use deno_runtime::code_cache;
@@ -58,10 +59,13 @@ use crate::cache::CodeCache;
5859
use crate::cache::FastInsecureHasher;
5960
use crate::cache::ParsedSourceCache;
6061
use crate::emit::Emitter;
62+
use crate::errors::get_module_error_class;
6163
use crate::graph_container::MainModuleGraphContainer;
6264
use crate::graph_container::ModuleGraphContainer;
6365
use crate::graph_container::ModuleGraphUpdatePermit;
66+
use crate::graph_util::enhance_graph_error;
6467
use crate::graph_util::CreateGraphOptions;
68+
use crate::graph_util::EnhanceGraphErrorMode;
6569
use crate::graph_util::ModuleGraphBuilder;
6670
use crate::node::CliNodeCodeTranslator;
6771
use crate::node::CliNodeResolver;
@@ -703,7 +707,21 @@ impl<TGraphContainer: ModuleGraphContainer>
703707
unreachable!("Deno bug. {} was misconfigured internally.", specifier);
704708
}
705709

706-
match graph.get(specifier) {
710+
let maybe_module = match graph.try_get(specifier) {
711+
Ok(module) => module,
712+
Err(err) => {
713+
return Err(custom_error(
714+
get_module_error_class(err),
715+
enhance_graph_error(
716+
&self.shared.sys,
717+
&ModuleGraphError::ModuleError(err.clone()),
718+
EnhanceGraphErrorMode::ShowRange,
719+
),
720+
))
721+
}
722+
};
723+
724+
match maybe_module {
707725
Some(deno_graph::Module::Json(JsonModule {
708726
source,
709727
media_type,

0 commit comments

Comments
 (0)