From 2bf2438f389fd5695b26cec4c9a3232ab4bd25da Mon Sep 17 00:00:00 2001 From: Elian Cordoba Date: Fri, 14 Jul 2023 13:47:18 -0300 Subject: [PATCH] fix(npm): improve error message on directory import in npm package (#19538) Co-authored-by: David Sherret --- cli/module_loader.rs | 33 +++++++++++++++---- cli/tests/integration/npm_tests.rs | 16 +++++++++ .../npm/directory_import/folder_index_js.out | 7 ++++ .../npm/directory_import/folder_index_js.ts | 2 ++ .../npm/directory_import/folder_no_index.out | 6 ++++ .../npm/directory_import/folder_no_index.ts | 2 ++ .../1.0.0/folder_index_js/index.d.ts | 1 + .../1.0.0/folder_index_js/index.js | 3 ++ .../1.0.0/folder_no_index/random_name.js | 1 + .../@denotest/sub-folders/1.0.0/main.mjs | 3 ++ .../@denotest/sub-folders/1.0.0/package.json | 6 ++++ 11 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 cli/tests/testdata/npm/directory_import/folder_index_js.out create mode 100644 cli/tests/testdata/npm/directory_import/folder_index_js.ts create mode 100644 cli/tests/testdata/npm/directory_import/folder_no_index.out create mode 100644 cli/tests/testdata/npm/directory_import/folder_no_index.ts create mode 100644 cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.d.ts create mode 100644 cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_no_index/random_name.js create mode 100644 cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/main.mjs create mode 100644 cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/package.json diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 9e814662c1..54efcd9dd8 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -748,13 +748,34 @@ impl NpmModuleLoader { .read_to_string(&file_path) .map_err(AnyError::from) .with_context(|| { - let mut msg = "Unable to load ".to_string(); - msg.push_str(&file_path.to_string_lossy()); - if let Some(referrer) = &maybe_referrer { - msg.push_str(" imported from "); - msg.push_str(referrer.as_str()); + if file_path.is_dir() { + // directory imports are not allowed when importing from an + // ES module, so provide the user with a helpful error message + let dir_path = file_path; + let mut msg = "Directory import ".to_string(); + msg.push_str(&dir_path.to_string_lossy()); + if let Some(referrer) = &maybe_referrer { + msg.push_str(" is not supported resolving import from "); + msg.push_str(referrer.as_str()); + let entrypoint_name = ["index.mjs", "index.js", "index.cjs"] + .iter() + .find(|e| dir_path.join(e).is_file()); + if let Some(entrypoint_name) = entrypoint_name { + msg.push_str("\nDid you mean to import "); + msg.push_str(entrypoint_name); + msg.push_str(" within the directory?"); + } + } + msg + } else { + let mut msg = "Unable to load ".to_string(); + msg.push_str(&file_path.to_string_lossy()); + if let Some(referrer) = &maybe_referrer { + msg.push_str(" imported from "); + msg.push_str(referrer.as_str()); + } + msg } - msg })?; let code = if self.cjs_resolutions.contains(specifier) { diff --git a/cli/tests/integration/npm_tests.rs b/cli/tests/integration/npm_tests.rs index 6e35456bf4..6da402a385 100644 --- a/cli/tests/integration/npm_tests.rs +++ b/cli/tests/integration/npm_tests.rs @@ -778,6 +778,22 @@ itest!(deno_run_non_existent { exit_code: 1, }); +itest!(directory_import_folder_index_js { + args: "run npm/directory_import/folder_index_js.ts", + output: "npm/directory_import/folder_index_js.out", + envs: env_vars_for_npm_tests(), + http_server: true, + exit_code: 1, +}); + +itest!(directory_import_folder_no_index { + args: "run npm/directory_import/folder_no_index.ts", + output: "npm/directory_import/folder_no_index.out", + envs: env_vars_for_npm_tests(), + http_server: true, + exit_code: 1, +}); + itest!(builtin_module_module { args: "run --allow-read --quiet npm/builtin_module_module/main.js", output: "npm/builtin_module_module/main.out", diff --git a/cli/tests/testdata/npm/directory_import/folder_index_js.out b/cli/tests/testdata/npm/directory_import/folder_index_js.out new file mode 100644 index 0000000000..72f285fc01 --- /dev/null +++ b/cli/tests/testdata/npm/directory_import/folder_index_js.out @@ -0,0 +1,7 @@ +Download http://localhost:4545/npm/registry/@denotest/sub-folders +Download http://localhost:4545/npm/registry/@denotest/sub-folders/1.0.0.tgz +error: Directory import [WILDCARD]folder_index_js is not supported resolving import from file:///[WILDCARD]/directory_import/folder_index_js.ts +Did you mean to import index.js within the directory? + +Caused by: + [WILDCARD] diff --git a/cli/tests/testdata/npm/directory_import/folder_index_js.ts b/cli/tests/testdata/npm/directory_import/folder_index_js.ts new file mode 100644 index 0000000000..b0d51fcd97 --- /dev/null +++ b/cli/tests/testdata/npm/directory_import/folder_index_js.ts @@ -0,0 +1,2 @@ +import test from "npm:@denotest/sub-folders/folder_index_js"; +console.log(test); diff --git a/cli/tests/testdata/npm/directory_import/folder_no_index.out b/cli/tests/testdata/npm/directory_import/folder_no_index.out new file mode 100644 index 0000000000..86ac88207b --- /dev/null +++ b/cli/tests/testdata/npm/directory_import/folder_no_index.out @@ -0,0 +1,6 @@ +Download http://localhost:4545/npm/registry/@denotest/sub-folders +Download http://localhost:4545/npm/registry/@denotest/sub-folders/1.0.0.tgz +error: Directory import [WILDCARD]folder_no_index is not supported resolving import from file:///[WILDCARD]/folder_no_index.ts + +Caused by: + [WILDCARD] diff --git a/cli/tests/testdata/npm/directory_import/folder_no_index.ts b/cli/tests/testdata/npm/directory_import/folder_no_index.ts new file mode 100644 index 0000000000..4c5fb7ec09 --- /dev/null +++ b/cli/tests/testdata/npm/directory_import/folder_no_index.ts @@ -0,0 +1,2 @@ +import test from "npm:@denotest/sub-folders/folder_no_index"; +console.log(test); diff --git a/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.d.ts b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.d.ts new file mode 100644 index 0000000000..c3ec6ac2e6 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.d.ts @@ -0,0 +1 @@ +export function add(a, b): number; diff --git a/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.js b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.js new file mode 100644 index 0000000000..71a2da49a1 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_index_js/index.js @@ -0,0 +1,3 @@ +export function add(a, b) { + return a + b; +} diff --git a/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_no_index/random_name.js b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_no_index/random_name.js new file mode 100644 index 0000000000..f4e8d9d29a --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/folder_no_index/random_name.js @@ -0,0 +1 @@ +module.exports = 5; diff --git a/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/main.mjs b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/main.mjs new file mode 100644 index 0000000000..358b4b09e5 --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/main.mjs @@ -0,0 +1,3 @@ +export function getValue() { + return 5; +} diff --git a/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/package.json b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/package.json new file mode 100644 index 0000000000..1402e346cb --- /dev/null +++ b/cli/tests/testdata/npm/registry/@denotest/sub-folders/1.0.0/package.json @@ -0,0 +1,6 @@ +{ + "name": "@denotest/sub-folders", + "version": "1.0.0", + "type": "module", + "main": "main.mjs" +}