From 21e8260cc9a8124c57e0bf7e829f07984f1de1ff Mon Sep 17 00:00:00 2001 From: Marvin Hagemeister Date: Mon, 27 Jan 2025 17:54:35 +0100 Subject: [PATCH 1/4] fix(lint): update jsx/react related rules and names (#27836) This PR updates `deno_lint` which contains a couple of bug fixes for JSX/React related rules. The react rules now have all a `react-*` prefix in the name as well. --- Cargo.lock | 4 ++-- cli/Cargo.toml | 2 +- cli/schemas/lint-rules.v1.json | 8 +++---- tests/specs/lint/bom/mod.out | 2 +- tests/specs/lint/gitignore/expected.out | 2 +- tests/specs/lint/jsr_tag/package.out | 2 +- tests/specs/lint/jsx/react-jsx.out | 2 +- .../lint.out | 4 ++-- .../main_unix.out | 4 ++-- .../main_windows.out | 4 ++-- tests/specs/lint/quiet/expected_quiet.out | 4 ++-- .../specs/lint/stdin/expected_from_stdin.out | 2 +- .../lint/syntax_error_reporting/lint.out | 2 +- tests/specs/lint/with_config/with_config.out | 4 ++-- .../with_config_and_flags.out | 4 ++-- .../with_config_without_tags.out | 4 ++-- .../with_glob_config_unix.out | 18 +++++++-------- .../with_glob_config_windows.out | 18 +++++++-------- .../with_glob_config_and_flags_unix.out | 22 +++++++++---------- .../with_glob_config_and_flags_windows.out | 22 +++++++++---------- tests/specs/lint/workspace/a.out | 6 ++--- tests/specs/lint/workspace/root.out | 16 +++++++------- 22 files changed, 78 insertions(+), 78 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 030af392b4..9f287ecfc7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1945,9 +1945,9 @@ dependencies = [ [[package]] name = "deno_lint" -version = "0.69.0" +version = "0.70.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "802583d3ca6c7063e14cafa02ddc206fb34e804e095d52032baf375c56a99515" +checksum = "ac94db8d8597b96c92d30a68b11d4bec6822dcbb3e8675ab1e0136816a301a34" dependencies = [ "anyhow", "deno_ast", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 580420ede1..32bf639250 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -74,7 +74,7 @@ deno_doc = { version = "=0.164.0", features = ["rust", "comrak"] } deno_error.workspace = true deno_graph = { version = "=0.87.0" } deno_lib.workspace = true -deno_lint = { version = "0.69.0" } +deno_lint = { version = "0.70.0" } deno_lockfile.workspace = true deno_media_type = { workspace = true, features = ["data_url", "decoding", "module_specifier"] } deno_npm.workspace = true diff --git a/cli/schemas/lint-rules.v1.json b/cli/schemas/lint-rules.v1.json index cfec281952..87bd4e2600 100644 --- a/cli/schemas/lint-rules.v1.json +++ b/cli/schemas/lint-rules.v1.json @@ -8,7 +8,6 @@ "ban-untagged-ignore", "ban-untagged-todo", "ban-unused-ignore", - "button-has-type", "camelcase", "constructor-super", "default-param-last", @@ -21,11 +20,11 @@ "getter-return", "guard-for-in", "jsx-boolean-value", + "jsx-button-has-type", "jsx-curly-braces", "jsx-key", "jsx-no-children-prop", "jsx-no-comment-text-nodes", - "jsx-no-danger-with-children", "jsx-no-duplicate-props", "jsx-no-unescaped-entities", "jsx-no-useless-fragment", @@ -44,7 +43,6 @@ "no-const-assign", "no-constant-condition", "no-control-regex", - "no-danger", "no-debugger", "no-delete-var", "no-deprecated-deno-api", @@ -115,9 +113,11 @@ "prefer-const", "prefer-namespace-keyword", "prefer-primordials", + "react-no-danger", + "react-no-danger-with-children", + "react-rules-of-hooks", "require-await", "require-yield", - "rules-of-hooks", "single-var-declarator", "triple-slash-reference", "use-isnan", diff --git a/tests/specs/lint/bom/mod.out b/tests/specs/lint/bom/mod.out index eea1e531d7..18d9927958 100644 --- a/tests/specs/lint/bom/mod.out +++ b/tests/specs/lint/bom/mod.out @@ -5,7 +5,7 @@ error[no-unused-vars]: `t` is never used | ^ = hint: If this is intentional, prefix it with an underscore like `_t` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 1 problem diff --git a/tests/specs/lint/gitignore/expected.out b/tests/specs/lint/gitignore/expected.out index 8024b51d19..ac982926bd 100644 --- a/tests/specs/lint/gitignore/expected.out +++ b/tests/specs/lint/gitignore/expected.out @@ -5,7 +5,7 @@ error[no-empty]: Empty block statement | ^^ = hint: Add code or comment to the empty block - docs: https://lint.deno.land/rules/no-empty + docs: https://docs.deno.com/lint/rules/no-empty Found 1 problem diff --git a/tests/specs/lint/jsr_tag/package.out b/tests/specs/lint/jsr_tag/package.out index 5169d9815f..ff26e5104f 100644 --- a/tests/specs/lint/jsr_tag/package.out +++ b/tests/specs/lint/jsr_tag/package.out @@ -5,7 +5,7 @@ error[verbatim-module-syntax]: All import identifiers are used in types | ^^^^^^ = hint: Change `import` to `import type` and optionally add an explicit side effect import - docs: https://lint.deno.land/rules/verbatim-module-syntax + docs: https://docs.deno.com/lint/rules/verbatim-module-syntax Found 1 problem (1 fixable via --fix) diff --git a/tests/specs/lint/jsx/react-jsx.out b/tests/specs/lint/jsx/react-jsx.out index c8c7007791..3aba846e67 100644 --- a/tests/specs/lint/jsx/react-jsx.out +++ b/tests/specs/lint/jsx/react-jsx.out @@ -5,7 +5,7 @@ error[no-unused-vars]: `React` is never used | ^^^^^ = hint: If this is intentional, prefix it with an underscore like `_React` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 1 problem diff --git a/tests/specs/lint/node_globals_no_duplicate_imports/lint.out b/tests/specs/lint/node_globals_no_duplicate_imports/lint.out index d9ff7f77e2..7f2fcf7860 100644 --- a/tests/specs/lint/node_globals_no_duplicate_imports/lint.out +++ b/tests/specs/lint/node_globals_no_duplicate_imports/lint.out @@ -5,7 +5,7 @@ error[no-node-globals]: NodeJS globals are not available in Deno | ^^^^^^^^^^^^ = hint: Add `import { setImmediate } from "node:timers";` - docs: https://lint.deno.land/rules/no-node-globals + docs: https://docs.deno.com/lint/rules/no-node-globals error[no-node-globals]: NodeJS globals are not available in Deno @@ -15,7 +15,7 @@ error[no-node-globals]: NodeJS globals are not available in Deno | ^^^^^^^^^^^^ = hint: Add `import { setImmediate } from "node:timers";` - docs: https://lint.deno.land/rules/no-node-globals + docs: https://docs.deno.com/lint/rules/no-node-globals Found 2 problems (2 fixable via --fix) diff --git a/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_unix.out b/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_unix.out index 39a3e7746c..c603aab53a 100644 --- a/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_unix.out +++ b/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_unix.out @@ -6,7 +6,7 @@ error[no-unused-vars]: `a` is never used | ^ = hint: If this is intentional, prefix it with an underscore like `_a` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `a` is never used @@ -16,7 +16,7 @@ error[no-unused-vars]: `a` is never used | ^ = hint: If this is intentional, prefix it with an underscore like `_a` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars [UNORDERED_END] diff --git a/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_windows.out b/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_windows.out index 8edf1133ea..ccd4faaad7 100644 --- a/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_windows.out +++ b/tests/specs/lint/opt_out_top_level_exclude_via_lint_inexclude/main_windows.out @@ -6,7 +6,7 @@ error[no-unused-vars]: `a` is never used | ^ = hint: If this is intentional, prefix it with an underscore like `_a` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `a` is never used @@ -16,7 +16,7 @@ error[no-unused-vars]: `a` is never used | ^ = hint: If this is intentional, prefix it with an underscore like `_a` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars [UNORDERED_END] diff --git a/tests/specs/lint/quiet/expected_quiet.out b/tests/specs/lint/quiet/expected_quiet.out index 91c1a29cf1..3e154a49fd 100644 --- a/tests/specs/lint/quiet/expected_quiet.out +++ b/tests/specs/lint/quiet/expected_quiet.out @@ -5,7 +5,7 @@ error[ban-untagged-ignore]: Ignore directive requires lint rule name(s) | ^^^^^^^^^^^^^^^^^^^ = hint: Add one or more lint rule names. E.g. // deno-lint-ignore adjacent-overload-signatures - docs: https://lint.deno.land/rules/ban-untagged-ignore + docs: https://docs.deno.com/lint/rules/ban-untagged-ignore error[no-empty]: Empty block statement @@ -15,6 +15,6 @@ error[no-empty]: Empty block statement | ^^ = hint: Add code or comment to the empty block - docs: https://lint.deno.land/rules/no-empty + docs: https://docs.deno.com/lint/rules/no-empty diff --git a/tests/specs/lint/stdin/expected_from_stdin.out b/tests/specs/lint/stdin/expected_from_stdin.out index f65331ebd9..58f0804481 100644 --- a/tests/specs/lint/stdin/expected_from_stdin.out +++ b/tests/specs/lint/stdin/expected_from_stdin.out @@ -5,7 +5,7 @@ error[no-explicit-any]: `any` type is not allowed | ^^^ = hint: Use a specific type other than `any` - docs: https://lint.deno.land/rules/no-explicit-any + docs: https://docs.deno.com/lint/rules/no-explicit-any Found 1 problem diff --git a/tests/specs/lint/syntax_error_reporting/lint.out b/tests/specs/lint/syntax_error_reporting/lint.out index 31e4c576b6..c317faa205 100644 --- a/tests/specs/lint/syntax_error_reporting/lint.out +++ b/tests/specs/lint/syntax_error_reporting/lint.out @@ -9,7 +9,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 1 problem diff --git a/tests/specs/lint/with_config/with_config.out b/tests/specs/lint/with_config/with_config.out index f527bb7121..caccc81aba 100644 --- a/tests/specs/lint/with_config/with_config.out +++ b/tests/specs/lint/with_config/with_config.out @@ -5,7 +5,7 @@ error[ban-untagged-todo]: TODO should be tagged with (@username) or (#issue) | ^^^^^^^^^^^^ = hint: Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123) - docs: https://lint.deno.land/rules/ban-untagged-todo + docs: https://docs.deno.com/lint/rules/ban-untagged-todo error[no-unused-vars]: `add` is never used @@ -15,7 +15,7 @@ error[no-unused-vars]: `add` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_add` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 2 problems diff --git a/tests/specs/lint/with_config_and_flags/with_config_and_flags.out b/tests/specs/lint/with_config_and_flags/with_config_and_flags.out index 78e21ef8d8..c42ecbe3d8 100644 --- a/tests/specs/lint/with_config_and_flags/with_config_and_flags.out +++ b/tests/specs/lint/with_config_and_flags/with_config_and_flags.out @@ -5,7 +5,7 @@ error[ban-untagged-todo]: TODO should be tagged with (@username) or (#issue) | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = hint: Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123) - docs: https://lint.deno.land/rules/ban-untagged-todo + docs: https://docs.deno.com/lint/rules/ban-untagged-todo error[no-unused-vars]: `subtract` is never used @@ -15,7 +15,7 @@ error[no-unused-vars]: `subtract` is never used | ^^^^^^^^ = hint: If this is intentional, prefix it with an underscore like `_subtract` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 2 problems diff --git a/tests/specs/lint/with_config_without_args/with_config_without_tags.out b/tests/specs/lint/with_config_without_args/with_config_without_tags.out index f527bb7121..caccc81aba 100644 --- a/tests/specs/lint/with_config_without_args/with_config_without_tags.out +++ b/tests/specs/lint/with_config_without_args/with_config_without_tags.out @@ -5,7 +5,7 @@ error[ban-untagged-todo]: TODO should be tagged with (@username) or (#issue) | ^^^^^^^^^^^^ = hint: Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123) - docs: https://lint.deno.land/rules/ban-untagged-todo + docs: https://docs.deno.com/lint/rules/ban-untagged-todo error[no-unused-vars]: `add` is never used @@ -15,7 +15,7 @@ error[no-unused-vars]: `add` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_add` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 2 problems diff --git a/tests/specs/lint/with_glob_config/with_glob_config_unix.out b/tests/specs/lint/with_glob_config/with_glob_config_unix.out index fad285cbe7..1224bd5f90 100644 --- a/tests/specs/lint/with_glob_config/with_glob_config_unix.out +++ b/tests/specs/lint/with_glob_config/with_glob_config_unix.out @@ -6,7 +6,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -16,7 +16,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -26,7 +26,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -36,7 +36,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -46,7 +46,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -56,7 +56,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -66,7 +66,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -76,7 +76,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -86,7 +86,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars [UNORDERED_END] diff --git a/tests/specs/lint/with_glob_config/with_glob_config_windows.out b/tests/specs/lint/with_glob_config/with_glob_config_windows.out index 2ba0787912..a09ac13155 100644 --- a/tests/specs/lint/with_glob_config/with_glob_config_windows.out +++ b/tests/specs/lint/with_glob_config/with_glob_config_windows.out @@ -6,7 +6,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -16,7 +16,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -26,7 +26,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -36,7 +36,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -46,7 +46,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -56,7 +56,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -66,7 +66,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -76,7 +76,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -86,7 +86,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars [UNORDERED_END] diff --git a/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_unix.out b/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_unix.out index 9afca955ba..7e8cb919eb 100644 --- a/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_unix.out +++ b/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_unix.out @@ -6,7 +6,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -16,7 +16,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -26,7 +26,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -36,7 +36,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -46,7 +46,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -56,7 +56,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -66,7 +66,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -76,7 +76,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -86,7 +86,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -96,7 +96,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -106,7 +106,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars [UNORDERED_END] diff --git a/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_windows.out b/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_windows.out index 9446796ab9..3832a4a65c 100644 --- a/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_windows.out +++ b/tests/specs/lint/with_glob_config_and_flags/with_glob_config_and_flags_windows.out @@ -6,7 +6,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -16,7 +16,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -26,7 +26,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -36,7 +36,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -46,7 +46,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -56,7 +56,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -66,7 +66,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -76,7 +76,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -86,7 +86,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -96,7 +96,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-unused-vars]: `foo` is never used @@ -106,7 +106,7 @@ error[no-unused-vars]: `foo` is never used | ^^^ = hint: If this is intentional, prefix it with an underscore like `_foo` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars [UNORDERED_END] diff --git a/tests/specs/lint/workspace/a.out b/tests/specs/lint/workspace/a.out index 52f05af990..002fc886c3 100644 --- a/tests/specs/lint/workspace/a.out +++ b/tests/specs/lint/workspace/a.out @@ -5,7 +5,7 @@ error[no-eval]: `eval` call is not allowed | ^^^^^^^^ = hint: Remove the use of `eval` - docs: https://lint.deno.land/rules/no-eval + docs: https://docs.deno.com/lint/rules/no-eval error[no-await-in-loop]: Unexpected `await` inside a loop. @@ -15,7 +15,7 @@ error[no-await-in-loop]: Unexpected `await` inside a loop. | ^^^^^^^^^^^^^^^^^^^^^^^ = hint: Remove `await` in loop body, store all promises generated and then `await Promise.all(storedPromises)` after the loop - docs: https://lint.deno.land/rules/no-await-in-loop + docs: https://docs.deno.com/lint/rules/no-await-in-loop error[no-explicit-any]: `any` type is not allowed @@ -25,7 +25,7 @@ error[no-explicit-any]: `any` type is not allowed | ^^^ = hint: Use a specific type other than `any` - docs: https://lint.deno.land/rules/no-explicit-any + docs: https://docs.deno.com/lint/rules/no-explicit-any Found 3 problems diff --git a/tests/specs/lint/workspace/root.out b/tests/specs/lint/workspace/root.out index 1d892a93f0..e2db1a6e09 100644 --- a/tests/specs/lint/workspace/root.out +++ b/tests/specs/lint/workspace/root.out @@ -5,7 +5,7 @@ error[no-eval]: `eval` call is not allowed | ^^^^^^^^ = hint: Remove the use of `eval` - docs: https://lint.deno.land/rules/no-eval + docs: https://docs.deno.com/lint/rules/no-eval error[no-unused-vars]: `unused` is never used @@ -15,7 +15,7 @@ error[no-unused-vars]: `unused` is never used | ^^^^^^ = hint: If this is intentional, prefix it with an underscore like `_unused` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars error[no-explicit-any]: `any` type is not allowed @@ -25,7 +25,7 @@ error[no-explicit-any]: `any` type is not allowed | ^^^ = hint: Use a specific type other than `any` - docs: https://lint.deno.land/rules/no-explicit-any + docs: https://docs.deno.com/lint/rules/no-explicit-any error[no-eval]: `eval` call is not allowed @@ -35,7 +35,7 @@ error[no-eval]: `eval` call is not allowed | ^^^^^^^^ = hint: Remove the use of `eval` - docs: https://lint.deno.land/rules/no-eval + docs: https://docs.deno.com/lint/rules/no-eval error[no-await-in-loop]: Unexpected `await` inside a loop. @@ -45,7 +45,7 @@ error[no-await-in-loop]: Unexpected `await` inside a loop. | ^^^^^^^^^^^^^^^^^^^^^^^ = hint: Remove `await` in loop body, store all promises generated and then `await Promise.all(storedPromises)` after the loop - docs: https://lint.deno.land/rules/no-await-in-loop + docs: https://docs.deno.com/lint/rules/no-await-in-loop error[no-explicit-any]: `any` type is not allowed @@ -55,7 +55,7 @@ error[no-explicit-any]: `any` type is not allowed | ^^^ = hint: Use a specific type other than `any` - docs: https://lint.deno.land/rules/no-explicit-any + docs: https://docs.deno.com/lint/rules/no-explicit-any error[no-eval]: `eval` call is not allowed @@ -65,7 +65,7 @@ error[no-eval]: `eval` call is not allowed | ^^^^^^^^ = hint: Remove the use of `eval` - docs: https://lint.deno.land/rules/no-eval + docs: https://docs.deno.com/lint/rules/no-eval error[no-unused-vars]: `unused` is never used @@ -75,7 +75,7 @@ error[no-unused-vars]: `unused` is never used | ^^^^^^ = hint: If this is intentional, prefix it with an underscore like `_unused` - docs: https://lint.deno.land/rules/no-unused-vars + docs: https://docs.deno.com/lint/rules/no-unused-vars Found 8 problems From 88490d092751288f736855b2418a4da606a31ce7 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Mon, 27 Jan 2025 22:25:00 +0530 Subject: [PATCH 2/4] fix(ext/crypto): export private x25519 JWK key (#27828) Ref https://github.com/denoland/deno/issues/26431 --- ext/crypto/00_crypto.js | 12 +++++++----- ext/crypto/lib.rs | 1 + ext/crypto/x25519.rs | 23 +++++++++++++++++------ tests/unit/webcrypto_test.ts | 17 +++++++++++++++++ tests/wpt/runner/expectation.json | 16 ---------------- 5 files changed, 42 insertions(+), 27 deletions(-) diff --git a/ext/crypto/00_crypto.js b/ext/crypto/00_crypto.js index 5a9f32cb00..e26d48506c 100644 --- a/ext/crypto/00_crypto.js +++ b/ext/crypto/00_crypto.js @@ -48,6 +48,7 @@ import { op_crypto_verify_ed25519, op_crypto_verify_key, op_crypto_wrap_key, + op_crypto_x25519_public_key, } from "ext:core/ops"; const { ArrayBufferIsView, @@ -4532,17 +4533,18 @@ function exportKeyX25519(format, key, innerKey) { return TypedArrayPrototypeGetBuffer(pkcs8Der); } case "jwk": { - if (key[_type] === "private") { - throw new DOMException("Not implemented", "NotSupportedError"); - } - const x = op_crypto_base64url_encode(innerKey); const jwk = { kty: "OKP", crv: "X25519", - x, "key_ops": key.usages, ext: key[_extractable], }; + if (key[_type] === "private") { + jwk.x = op_crypto_x25519_public_key(innerKey); + jwk.d = op_crypto_base64url_encode(innerKey); + } else { + jwk.x = op_crypto_base64url_encode(innerKey); + } return jwk; } default: diff --git a/ext/crypto/lib.rs b/ext/crypto/lib.rs index 0d6eecb911..9a5b71500b 100644 --- a/ext/crypto/lib.rs +++ b/ext/crypto/lib.rs @@ -98,6 +98,7 @@ deno_core::extension!(deno_crypto, op_crypto_base64url_decode, op_crypto_base64url_encode, x25519::op_crypto_generate_x25519_keypair, + x25519::op_crypto_x25519_public_key, x25519::op_crypto_derive_bits_x25519, x25519::op_crypto_import_spki_x25519, x25519::op_crypto_import_pkcs8_x25519, diff --git a/ext/crypto/x25519.rs b/ext/crypto/x25519.rs index 226ed89e40..aee8ae0e29 100644 --- a/ext/crypto/x25519.rs +++ b/ext/crypto/x25519.rs @@ -1,5 +1,6 @@ // Copyright 2018-2025 the Deno authors. MIT license. +use base64::prelude::BASE64_URL_SAFE_NO_PAD; use curve25519_dalek::montgomery::MontgomeryPoint; use deno_core::op2; use deno_core::ToJsBuffer; @@ -20,17 +21,16 @@ pub enum X25519Error { #[error(transparent)] Der(#[from] spki::der::Error), } - +// u-coordinate of the base point. +const X25519_BASEPOINT_BYTES: [u8; 32] = [ + 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, +]; #[op2(fast)] pub fn op_crypto_generate_x25519_keypair( #[buffer] pkey: &mut [u8], #[buffer] pubkey: &mut [u8], ) { - // u-coordinate of the base point. - const X25519_BASEPOINT_BYTES: [u8; 32] = [ - 9, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, - ]; let mut rng = OsRng; rng.fill_bytes(pkey); // https://www.rfc-editor.org/rfc/rfc7748#section-6.1 @@ -42,6 +42,17 @@ pub fn op_crypto_generate_x25519_keypair( pubkey.copy_from_slice(&x25519_dalek::x25519(pkey, X25519_BASEPOINT_BYTES)); } +#[op2] +#[string] +pub fn op_crypto_x25519_public_key(#[buffer] private_key: &[u8]) -> String { + use base64::Engine; + + let private_key: [u8; 32] = + private_key.try_into().expect("Expected byteLength 32"); + BASE64_URL_SAFE_NO_PAD + .encode(x25519_dalek::x25519(private_key, X25519_BASEPOINT_BYTES)) +} + const MONTGOMERY_IDENTITY: MontgomeryPoint = MontgomeryPoint([0; 32]); #[op2(fast)] diff --git a/tests/unit/webcrypto_test.ts b/tests/unit/webcrypto_test.ts index bc5b307de5..1732bb2635 100644 --- a/tests/unit/webcrypto_test.ts +++ b/tests/unit/webcrypto_test.ts @@ -2085,3 +2085,20 @@ Deno.test(async function x25519SharedSecret() { assertEquals(sharedSecret1.byteLength, 16); assertEquals(new Uint8Array(sharedSecret1), new Uint8Array(sharedSecret2)); }); + +Deno.test(async function x25519ExportJwk() { + const keyPair = await crypto.subtle.generateKey( + { + name: "X25519", + }, + true, + ["deriveBits"], + ) as CryptoKeyPair; + + const jwk = await crypto.subtle.exportKey("jwk", keyPair.privateKey); + + assertEquals(jwk.kty, "OKP"); + assertEquals(jwk.crv, "X25519"); + assert(jwk.d); + assert(jwk.x); +}); diff --git a/tests/wpt/runner/expectation.json b/tests/wpt/runner/expectation.json index 6d2cc38562..01a4e0b5e9 100644 --- a/tests/wpt/runner/expectation.json +++ b/tests/wpt/runner/expectation.json @@ -845,14 +845,6 @@ "Good parameters: Ed448 bits (jwk, object(crv, d, x, kty), {name: Ed448}, false, [sign])", "Good parameters: Ed448 bits (pkcs8, buffer(73), {name: Ed448}, false, [sign, sign])", "Good parameters: Ed448 bits (jwk, object(crv, d, x, kty), {name: Ed448}, false, [sign, sign])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits, deriveKey])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits, deriveKey])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey, deriveBits, deriveKey, deriveBits])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey, deriveBits, deriveKey, deriveBits])", "Good parameters: X448 bits (pkcs8, buffer(72), {name: X448}, true, [deriveKey])", "Good parameters: X448 bits (jwk, object(crv, d, x, kty), {name: X448}, true, [deriveKey])", "Good parameters with ignored JWK alg: X448 (jwk, object(crv, d, x, kty), {name: X448}, true, [deriveKey])", @@ -902,14 +894,6 @@ "Good parameters: Ed448 bits (jwk, object(crv, d, x, kty), {name: Ed448}, false, [sign])", "Good parameters: Ed448 bits (pkcs8, buffer(73), {name: Ed448}, false, [sign, sign])", "Good parameters: Ed448 bits (jwk, object(crv, d, x, kty), {name: Ed448}, false, [sign, sign])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits, deriveKey])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits, deriveKey])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveBits])", - "Good parameters: X25519 bits (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey, deriveBits, deriveKey, deriveBits])", - "Good parameters with ignored JWK alg: X25519 (jwk, object(crv, d, x, kty), {name: X25519}, true, [deriveKey, deriveBits, deriveKey, deriveBits])", "Good parameters: X448 bits (pkcs8, buffer(72), {name: X448}, true, [deriveKey])", "Good parameters: X448 bits (jwk, object(crv, d, x, kty), {name: X448}, true, [deriveKey])", "Good parameters with ignored JWK alg: X448 (jwk, object(crv, d, x, kty), {name: X448}, true, [deriveKey])", From 92dce12af7f559859059a6c84b7836dd08af0acc Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 27 Jan 2025 14:18:27 -0500 Subject: [PATCH 3/4] fix(install/global): warn about not including auto-discovered config file (#27745) Closes #17855 --- cli/tools/installer.rs | 6 ++++++ tests/specs/install/global/warn_config_file/__test__.jsonc | 5 +++++ tests/specs/install/global/warn_config_file/deno.json | 2 ++ tests/specs/install/global/warn_config_file/install.out | 3 +++ tests/specs/install/global/warn_config_file/main.js | 1 + 5 files changed, 17 insertions(+) create mode 100644 tests/specs/install/global/warn_config_file/__test__.jsonc create mode 100644 tests/specs/install/global/warn_config_file/deno.json create mode 100644 tests/specs/install/global/warn_config_file/install.out create mode 100644 tests/specs/install/global/warn_config_file/main.js diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 53429ef893..e4c7c66dda 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -409,6 +409,12 @@ async fn install_global( .load_and_type_check_files(&[install_flags_global.module_url.clone()]) .await?; + if matches!(flags.config_flag, ConfigFlag::Discover) + && cli_options.workspace().deno_jsons().next().is_some() + { + log::warn!("{} discovered config file will be ignored in the installed command. Use the --config flag if you wish to include it.", crate::colors::yellow("Warning")); + } + // create the install shim create_install_shim(http_client, &flags, install_flags_global).await } diff --git a/tests/specs/install/global/warn_config_file/__test__.jsonc b/tests/specs/install/global/warn_config_file/__test__.jsonc new file mode 100644 index 0000000000..1d36d4c143 --- /dev/null +++ b/tests/specs/install/global/warn_config_file/__test__.jsonc @@ -0,0 +1,5 @@ +{ + "tempDir": true, + "args": "install --root ./bins -g --name my-cli main.js", + "output": "install.out" +} diff --git a/tests/specs/install/global/warn_config_file/deno.json b/tests/specs/install/global/warn_config_file/deno.json new file mode 100644 index 0000000000..2c63c08510 --- /dev/null +++ b/tests/specs/install/global/warn_config_file/deno.json @@ -0,0 +1,2 @@ +{ +} diff --git a/tests/specs/install/global/warn_config_file/install.out b/tests/specs/install/global/warn_config_file/install.out new file mode 100644 index 0000000000..4ce57be390 --- /dev/null +++ b/tests/specs/install/global/warn_config_file/install.out @@ -0,0 +1,3 @@ +Warning discovered config file will be ignored in the installed command. Use the --config flag if you wish to include it. +✅ Successfully installed my-cli +[WILDCARD] diff --git a/tests/specs/install/global/warn_config_file/main.js b/tests/specs/install/global/warn_config_file/main.js new file mode 100644 index 0000000000..296d5492b0 --- /dev/null +++ b/tests/specs/install/global/warn_config_file/main.js @@ -0,0 +1 @@ +console.log(1); From 679902a108e21630183ae9199f8fdda868b99227 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 27 Jan 2025 15:23:20 -0500 Subject: [PATCH 4/4] perf(node_resolver): reduce url to/from path conversions (#27839) Extracted out of https://github.com/denoland/deno/pull/27838/files Reduces some allocations by accepting either a pathbuf or url for the referrer for resolution and returning either a pathbuf or url at the end, which the caller can then convert into to their preferred state. This is about 4% faster when still converting the final result to a url and 6% faster when keeping the result as a path in a benchmark I ran. --- Cargo.lock | 1 + cli/lib/worker.rs | 34 ++- cli/lsp/analysis.rs | 4 +- cli/lsp/resolver.rs | 8 +- cli/module_loader.rs | 9 +- cli/npm/installer/local.rs | 80 +++--- cli/rt/run.rs | 48 ++-- cli/tools/check.rs | 2 +- cli/tools/info.rs | 36 +-- cli/tsc/mod.rs | 14 +- ext/node/ops/require.rs | 53 ++-- resolvers/deno/cjs.rs | 10 +- resolvers/deno/factory.rs | 2 +- resolvers/deno/lib.rs | 49 ++-- resolvers/deno/npm/byonm.rs | 29 ++- resolvers/deno/npm/managed/common.rs | 3 +- resolvers/deno/npm/managed/global.rs | 15 +- resolvers/deno/npm/managed/local.rs | 19 +- resolvers/deno/npm/managed/mod.rs | 5 +- resolvers/deno/npm/mod.rs | 22 +- resolvers/node/Cargo.toml | 1 + resolvers/node/analyze.rs | 102 ++++---- resolvers/node/errors.rs | 60 ++++- resolvers/node/lib.rs | 2 + resolvers/node/npm.rs | 3 +- resolvers/node/package_json.rs | 21 +- resolvers/node/path.rs | 116 +++++++++ resolvers/node/resolution.rs | 348 ++++++++++++++------------- 28 files changed, 679 insertions(+), 417 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9f287ecfc7..93bc6bbb65 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5260,6 +5260,7 @@ dependencies = [ "anyhow", "async-trait", "boxed_error", + "dashmap", "deno_error", "deno_package_json", "deno_path_util", diff --git a/cli/lib/worker.rs b/cli/lib/worker.rs index 01862bfd82..a8c5528677 100644 --- a/cli/lib/worker.rs +++ b/cli/lib/worker.rs @@ -7,6 +7,8 @@ use std::sync::Arc; use deno_core::error::JsError; use deno_node::NodeRequireLoaderRc; +use deno_path_util::url_from_file_path; +use deno_path_util::url_to_file_path; use deno_resolver::npm::DenoInNpmPackageChecker; use deno_resolver::npm::NpmResolver; use deno_runtime::colors; @@ -44,6 +46,7 @@ use deno_runtime::WorkerExecutionMode; use deno_runtime::WorkerLogLevel; use deno_runtime::UNSTABLE_GRANULAR_FLAGS; use node_resolver::errors::ResolvePkgJsonBinExportError; +use node_resolver::UrlOrPath; use url::Url; use crate::args::has_trace_permissions_enabled; @@ -135,6 +138,9 @@ pub fn create_isolate_create_params() -> Option { #[derive(Debug, thiserror::Error, deno_error::JsError)] pub enum ResolveNpmBinaryEntrypointError { + #[class(inherit)] + #[error(transparent)] + PathToUrl(#[from] deno_path_util::PathToUrlError), #[class(inherit)] #[error(transparent)] ResolvePkgJsonBinExport(ResolvePkgJsonBinExportError), @@ -153,7 +159,7 @@ pub enum ResolveNpmBinaryEntrypointFallbackError { PackageSubpathResolve(node_resolver::errors::PackageSubpathResolveError), #[class(generic)] #[error("Cannot find module '{0}'")] - ModuleNotFound(Url), + ModuleNotFound(UrlOrPath), } pub struct LibMainWorkerOptions { @@ -525,13 +531,13 @@ impl LibMainWorkerFactory { .node_resolver .resolve_binary_export(package_folder, sub_path) { - Ok(specifier) => Ok(specifier), + Ok(path) => Ok(url_from_file_path(&path)?), Err(original_err) => { // if the binary entrypoint was not found, fallback to regular node resolution let result = self.resolve_binary_entrypoint_fallback(package_folder, sub_path); match result { - Ok(Some(specifier)) => Ok(specifier), + Ok(Some(path)) => Ok(url_from_file_path(&path)?), Ok(None) => { Err(ResolveNpmBinaryEntrypointError::ResolvePkgJsonBinExport( original_err, @@ -551,7 +557,7 @@ impl LibMainWorkerFactory { &self, package_folder: &Path, sub_path: Option<&str>, - ) -> Result, ResolveNpmBinaryEntrypointFallbackError> { + ) -> Result, ResolveNpmBinaryEntrypointFallbackError> { // only fallback if the user specified a sub path if sub_path.is_none() { // it's confusing to users if the package doesn't have any binary @@ -573,14 +579,22 @@ impl LibMainWorkerFactory { .map_err( ResolveNpmBinaryEntrypointFallbackError::PackageSubpathResolve, )?; - if deno_path_util::url_to_file_path(&specifier) - .map(|p| self.shared.sys.fs_exists_no_err(p)) - .unwrap_or(false) - { - Ok(Some(specifier)) + let path = match specifier { + UrlOrPath::Url(ref url) => match url_to_file_path(url) { + Ok(path) => path, + Err(_) => { + return Err(ResolveNpmBinaryEntrypointFallbackError::ModuleNotFound( + specifier, + )); + } + }, + UrlOrPath::Path(path) => path, + }; + if self.shared.sys.fs_exists_no_err(&path) { + Ok(Some(path)) } else { Err(ResolveNpmBinaryEntrypointFallbackError::ModuleNotFound( - specifier, + UrlOrPath::Path(path), )) } } diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index f8f382f594..44e9bf4dfd 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -449,9 +449,7 @@ impl<'a> TsResponseImportMapper<'a> { .pkg_json_resolver(specifier) // the specifier might have a closer package.json, but we // want the root of the package's package.json - .get_closest_package_json_from_file_path( - &package_root_folder.join("package.json"), - ) + .get_closest_package_json(&package_root_folder.join("package.json")) .ok() .flatten()?; let root_folder = package_json.path.parent()?; diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 13b4c73fff..7f544719a7 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -207,6 +207,8 @@ impl LspScopeResolver { NodeResolutionKind::Execution, ) }) + .ok()? + .into_url() .ok()?, )) .0; @@ -257,7 +259,7 @@ impl LspScopeResolver { root_node_modules_dir: byonm_npm_resolver .root_node_modules_path() .map(|p| p.to_path_buf()), - sys: CliSys::default(), + sys: factory.sys.clone(), pkg_json_resolver: self.pkg_json_resolver.clone(), }, ) @@ -522,6 +524,8 @@ impl LspResolver { resolution_mode, NodeResolutionKind::Types, ) + .ok()? + .into_url() .ok()?, ))) } @@ -702,7 +706,7 @@ impl<'a> ResolverFactory<'a> { let sys = CliSys::default(); let options = if enable_byonm { CliNpmResolverCreateOptions::Byonm(CliByonmNpmResolverCreateOptions { - sys, + sys: self.sys.clone(), pkg_json_resolver: self.pkg_json_resolver.clone(), root_node_modules_dir: self.config_data.and_then(|config_data| { config_data.node_modules_dir.clone().or_else(|| { diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 2e8713d5b6..7e023f0fa6 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -667,7 +667,12 @@ impl ResolutionMode::Import, NodeResolutionKind::Execution, ) - .map_err(|e| JsErrorBox::from_err(e).into()); + .map_err(|e| JsErrorBox::from_err(e).into()) + .and_then(|url_or_path| { + url_or_path + .into_url() + .map_err(|e| JsErrorBox::from_err(e).into()) + }); } } @@ -696,6 +701,8 @@ impl source, }) })? + .into_url() + .map_err(JsErrorBox::from_err)? } Some(Module::Node(module)) => module.specifier.clone(), Some(Module::Js(module)) => module.specifier.clone(), diff --git a/cli/npm/installer/local.rs b/cli/npm/installer/local.rs index 87288c6c8e..5e14b607f7 100644 --- a/cli/npm/installer/local.rs +++ b/cli/npm/installer/local.rs @@ -2,6 +2,7 @@ //! Code for local node_modules resolution. +use std::borrow::Cow; use std::cell::RefCell; use std::cmp::Ordering; use std::collections::hash_map::Entry; @@ -312,7 +313,7 @@ async fn sync_resolution_with_fs( ); let sub_node_modules = folder_path.join("node_modules"); let package_path = - join_package_name(&sub_node_modules, &package.id.nv.name); + join_package_name(Cow::Owned(sub_node_modules), &package.id.nv.name); let cache_folder = cache.package_folder_for_nv(&package.id.nv); deno_core::unsync::spawn_blocking({ @@ -350,7 +351,7 @@ async fn sync_resolution_with_fs( let sub_node_modules = folder_path.join("node_modules"); let package_path = - join_package_name(&sub_node_modules, &package.id.nv.name); + join_package_name(Cow::Owned(sub_node_modules), &package.id.nv.name); lifecycle_scripts.add(package, package_path.into()); } @@ -367,14 +368,16 @@ async fn sync_resolution_with_fs( if !initialized_file.exists() { let sub_node_modules = destination_path.join("node_modules"); let package_path = - join_package_name(&sub_node_modules, &package.id.nv.name); + join_package_name(Cow::Owned(sub_node_modules), &package.id.nv.name); let source_path = join_package_name( - &deno_local_registry_dir - .join(get_package_folder_id_folder_name( - &package_cache_folder_id.with_no_count(), - )) - .join("node_modules"), + Cow::Owned( + deno_local_registry_dir + .join(get_package_folder_id_folder_name( + &package_cache_folder_id.with_no_count(), + )) + .join("node_modules"), + ), &package.id.nv.name, ); @@ -407,14 +410,16 @@ async fn sync_resolution_with_fs( get_package_folder_id_folder_name(&dep_cache_folder_id); if dep_setup_cache.insert(name, &dep_folder_name) { let dep_folder_path = join_package_name( - &deno_local_registry_dir - .join(dep_folder_name) - .join("node_modules"), + Cow::Owned( + deno_local_registry_dir + .join(dep_folder_name) + .join("node_modules"), + ), &dep_id.nv.name, ); symlink_package_dir( &dep_folder_path, - &join_package_name(&sub_node_modules, name), + &join_package_name(Cow::Borrowed(&sub_node_modules), name), )?; } } @@ -468,9 +473,11 @@ async fn sync_resolution_with_fs( &remote_pkg.get_package_cache_folder_id(), ); let local_registry_package_path = join_package_name( - &deno_local_registry_dir - .join(&target_folder_name) - .join("node_modules"), + Cow::Owned( + deno_local_registry_dir + .join(&target_folder_name) + .join("node_modules"), + ), &remote_pkg.id.nv.name, ); if install_in_child { @@ -496,7 +503,10 @@ async fn sync_resolution_with_fs( { symlink_package_dir( &local_registry_package_path, - &join_package_name(root_node_modules_dir_path, remote_alias), + &join_package_name( + Cow::Borrowed(root_node_modules_dir_path), + remote_alias, + ), )?; } } @@ -526,15 +536,20 @@ async fn sync_resolution_with_fs( get_package_folder_id_folder_name(&package.get_package_cache_folder_id()); if setup_cache.insert_root_symlink(&id.nv.name, &target_folder_name) { let local_registry_package_path = join_package_name( - &deno_local_registry_dir - .join(target_folder_name) - .join("node_modules"), + Cow::Owned( + deno_local_registry_dir + .join(target_folder_name) + .join("node_modules"), + ), &id.nv.name, ); symlink_package_dir( &local_registry_package_path, - &join_package_name(root_node_modules_dir_path, &id.nv.name), + &join_package_name( + Cow::Borrowed(root_node_modules_dir_path), + &id.nv.name, + ), )?; } } @@ -556,15 +571,20 @@ async fn sync_resolution_with_fs( if setup_cache.insert_deno_symlink(&package.id.nv.name, &target_folder_name) { let local_registry_package_path = join_package_name( - &deno_local_registry_dir - .join(target_folder_name) - .join("node_modules"), + Cow::Owned( + deno_local_registry_dir + .join(target_folder_name) + .join("node_modules"), + ), &package.id.nv.name, ); symlink_package_dir( &local_registry_package_path, - &join_package_name(&deno_node_modules_dir, &package.id.nv.name), + &join_package_name( + Cow::Borrowed(&deno_node_modules_dir), + &package.id.nv.name, + ), )?; } } @@ -986,13 +1006,17 @@ fn junction_or_symlink_dir( } } -fn join_package_name(path: &Path, package_name: &str) -> PathBuf { - let mut path = path.to_path_buf(); +fn join_package_name(mut path: Cow, package_name: &str) -> PathBuf { // ensure backslashes are used on windows for part in package_name.split('/') { - path = path.join(part); + match path { + Cow::Borrowed(inner) => path = Cow::Owned(inner.join(part)), + Cow::Owned(ref mut path) => { + path.push(part); + } + } } - path + path.into_owned() } #[cfg(test)] diff --git a/cli/rt/run.rs b/cli/rt/run.rs index f106743c2a..53190f2433 100644 --- a/cli/rt/run.rs +++ b/cli/rt/run.rs @@ -196,8 +196,8 @@ impl ModuleLoader for EmbeddedModuleLoader { referrer_kind, NodeResolutionKind::Execution, ) - .map_err(JsErrorBox::from_err)? - .into_url(), + .and_then(|res| res.into_url()) + .map_err(JsErrorBox::from_err)?, ); } @@ -225,7 +225,10 @@ impl ModuleLoader for EmbeddedModuleLoader { referrer_kind, NodeResolutionKind::Execution, ) - .map_err(JsErrorBox::from_err)?, + .map_err(JsErrorBox::from_err) + .and_then(|url_or_path| { + url_or_path.into_url().map_err(JsErrorBox::from_err) + })?, ), Ok(MappedResolution::PackageJson { dep_result, @@ -236,17 +239,22 @@ impl ModuleLoader for EmbeddedModuleLoader { .as_ref() .map_err(|e| JsErrorBox::from_err(e.clone()))? { - PackageJsonDepValue::Req(req) => self - .shared - .npm_req_resolver - .resolve_req_with_sub_path( - req, - sub_path.as_deref(), - &referrer, - referrer_kind, - NodeResolutionKind::Execution, - ) - .map_err(|e| JsErrorBox::from_err(e).into()), + PackageJsonDepValue::Req(req) => Ok( + self + .shared + .npm_req_resolver + .resolve_req_with_sub_path( + req, + sub_path.as_deref(), + &referrer, + referrer_kind, + NodeResolutionKind::Execution, + ) + .map_err(JsErrorBox::from_err) + .and_then(|url_or_path| { + url_or_path.into_url().map_err(JsErrorBox::from_err) + })?, + ), PackageJsonDepValue::Workspace(version_req) => { let pkg_folder = self .shared @@ -267,7 +275,10 @@ impl ModuleLoader for EmbeddedModuleLoader { referrer_kind, NodeResolutionKind::Execution, ) - .map_err(JsErrorBox::from_err)?, + .map_err(JsErrorBox::from_err) + .and_then(|url_or_path| { + url_or_path.into_url().map_err(JsErrorBox::from_err) + })?, ) } }, @@ -286,7 +297,10 @@ impl ModuleLoader for EmbeddedModuleLoader { referrer_kind, NodeResolutionKind::Execution, ) - .map_err(JsErrorBox::from_err)?, + .map_err(JsErrorBox::from_err) + .and_then(|url_or_path| { + url_or_path.into_url().map_err(JsErrorBox::from_err) + })?, ); } @@ -323,7 +337,7 @@ impl ModuleLoader for EmbeddedModuleLoader { ) .map_err(JsErrorBox::from_err)?; if let Some(res) = maybe_res { - return Ok(res.into_url()); + return Ok(res.into_url().map_err(JsErrorBox::from_err)?); } Err(JsErrorBox::from_err(err).into()) } diff --git a/cli/tools/check.rs b/cli/tools/check.rs index d8128432b3..d3796cb740 100644 --- a/cli/tools/check.rs +++ b/cli/tools/check.rs @@ -667,7 +667,7 @@ fn resolve_npm_nv_ref( node_resolver::NodeResolutionKind::Types, ) .ok()?; - Some(resolved) + resolved.into_url().ok() } /// Matches the `@ts-check` pragma. diff --git a/cli/tools/info.rs b/cli/tools/info.rs index 765156793f..e297cd5b72 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -75,13 +75,17 @@ pub async fn info( target_pkg_json, sub_path, .. - } => Some(node_resolver.resolve_package_subpath_from_deno_module( - target_pkg_json.clone().dir_path(), - sub_path.as_deref(), - Some(&cwd_url), - node_resolver::ResolutionMode::Import, - node_resolver::NodeResolutionKind::Execution, - )?), + } => Some( + node_resolver + .resolve_package_subpath_from_deno_module( + target_pkg_json.clone().dir_path(), + sub_path.as_deref(), + Some(&cwd_url), + node_resolver::ResolutionMode::Import, + node_resolver::NodeResolutionKind::Execution, + )? + .into_url()?, + ), deno_config::workspace::MappedResolution::PackageJson { alias, sub_path, @@ -94,13 +98,17 @@ pub async fn info( alias, version_req, )?; - Some(node_resolver.resolve_package_subpath_from_deno_module( - pkg_folder, - sub_path.as_deref(), - Some(&cwd_url), - node_resolver::ResolutionMode::Import, - node_resolver::NodeResolutionKind::Execution, - )?) + Some( + node_resolver + .resolve_package_subpath_from_deno_module( + pkg_folder, + sub_path.as_deref(), + Some(&cwd_url), + node_resolver::ResolutionMode::Import, + node_resolver::NodeResolutionKind::Execution, + )? + .into_url()?, + ) } deno_package_json::PackageJsonDepValue::Req(req) => { Some(ModuleSpecifier::parse(&format!( diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index 1909f42a65..943c202af5 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -709,6 +709,9 @@ pub enum ResolveError { )] ModuleResolution(#[from] deno_core::ModuleResolutionError), #[class(inherit)] + #[error(transparent)] + FilePathToUrl(#[from] deno_path_util::PathToUrlError), + #[class(inherit)] #[error("{0}")] PackageSubpathResolve(PackageSubpathResolveError), #[class(inherit)] @@ -943,7 +946,7 @@ fn resolve_graph_specifier_types( NodeResolutionKind::Types, ); let maybe_url = match res_result { - Ok(url) => Some(url), + Ok(path_or_url) => Some(path_or_url.into_url()?), Err(err) => match err.code() { NodeJsErrorCode::ERR_TYPES_NOT_FOUND | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None, @@ -971,6 +974,9 @@ fn resolve_graph_specifier_types( #[derive(Debug, Error, deno_error::JsError)] pub enum ResolveNonGraphSpecifierTypesError { + #[class(inherit)] + #[error(transparent)] + FilePathToUrl(#[from] deno_path_util::PathToUrlError), #[class(inherit)] #[error(transparent)] ResolvePkgFolderFromDenoReq(#[from] ResolvePkgFolderFromDenoReqError), @@ -1003,8 +1009,8 @@ fn resolve_non_graph_specifier_types( resolution_mode, NodeResolutionKind::Types, ) - .ok() - .map(|res| res.into_url()), + .and_then(|res| res.into_url()) + .ok(), ))) } else if let Ok(npm_req_ref) = NpmPackageReqReference::from_str(raw_specifier) @@ -1025,7 +1031,7 @@ fn resolve_non_graph_specifier_types( NodeResolutionKind::Types, ); let maybe_url = match res_result { - Ok(url) => Some(url), + Ok(url_or_path) => Some(url_or_path.into_url()?), Err(err) => match err.code() { NodeJsErrorCode::ERR_TYPES_NOT_FOUND | NodeJsErrorCode::ERR_MODULE_NOT_FOUND => None, diff --git a/ext/node/ops/require.rs b/ext/node/ops/require.rs index c0e54993ae..53f57f0c68 100644 --- a/ext/node/ops/require.rs +++ b/ext/node/ops/require.rs @@ -23,6 +23,8 @@ use node_resolver::InNpmPackageChecker; use node_resolver::NodeResolutionKind; use node_resolver::NpmPackageFolderResolver; use node_resolver::ResolutionMode; +use node_resolver::UrlOrPath; +use node_resolver::UrlOrPathRef; use node_resolver::REQUIRE_CONDITIONS; use sys_traits::FsCanonicalize; use sys_traits::FsMetadata; @@ -277,11 +279,12 @@ pub fn op_require_resolve_deno_dir< TSys, >>(); + let path = PathBuf::from(parent_filename); Ok( resolver .resolve_package_folder_from_package( &request, - &url_from_file_path(&PathBuf::from(parent_filename))?, + &UrlOrPathRef::from_path(&path), ) .ok() .map(|p| p.to_string_lossy().into_owned()), @@ -487,9 +490,7 @@ pub fn op_require_try_self< let pkg_json_resolver = state.borrow::>(); let pkg = pkg_json_resolver - .get_closest_package_json_from_file_path(&PathBuf::from( - parent_path.unwrap(), - )) + .get_closest_package_json(&PathBuf::from(parent_path.unwrap())) .ok() .flatten(); if pkg.is_none() { @@ -515,13 +516,13 @@ pub fn op_require_try_self< return Ok(None); } - let referrer = deno_core::url::Url::from_file_path(&pkg.path).unwrap(); if let Some(exports) = &pkg.exports { let node_resolver = state.borrow::>(); + let referrer = UrlOrPathRef::from_path(&pkg.path); let r = node_resolver.package_exports_resolve( &pkg.path, &expansion, @@ -531,11 +532,7 @@ pub fn op_require_try_self< REQUIRE_CONDITIONS, NodeResolutionKind::Execution, )?; - Ok(Some(if r.scheme() == "file" { - url_to_file_path_string(&r)? - } else { - r.to_string() - })) + Ok(Some(url_or_path_to_string(r)?)) } else { Ok(None) } @@ -627,22 +624,21 @@ pub fn op_require_resolve_exports< let referrer = if parent_path.is_empty() { None } else { - Some(Url::from_file_path(parent_path).unwrap()) + Some(PathBuf::from(parent_path)) }; let r = node_resolver.package_exports_resolve( &pkg.path, &format!(".{expansion}"), exports, - referrer.as_ref(), + referrer + .as_ref() + .map(|r| UrlOrPathRef::from_path(r)) + .as_ref(), ResolutionMode::Require, REQUIRE_CONDITIONS, NodeResolutionKind::Execution, )?; - Ok(Some(if r.scheme() == "file" { - url_to_file_path_string(&r)? - } else { - r.to_string() - })) + Ok(Some(url_or_path_to_string(r)?)) } deno_error::js_error_wrapper!( @@ -701,8 +697,7 @@ pub fn op_require_package_imports_resolve< let referrer_path = ensure_read_permission::

(state, &referrer_path) .map_err(RequireErrorKind::Permission)?; let pkg_json_resolver = state.borrow::>(); - let Some(pkg) = pkg_json_resolver - .get_closest_package_json_from_file_path(&referrer_path)? + let Some(pkg) = pkg_json_resolver.get_closest_package_json(&referrer_path)? else { return Ok(None); }; @@ -713,16 +708,15 @@ pub fn op_require_package_imports_resolve< TNpmPackageFolderResolver, TSys, >>(); - let referrer_url = Url::from_file_path(&referrer_filename).unwrap(); let url = node_resolver.package_imports_resolve( &request, - Some(&referrer_url), + Some(&UrlOrPathRef::from_path(&referrer_path)), ResolutionMode::Require, Some(&pkg), REQUIRE_CONDITIONS, NodeResolutionKind::Execution, )?; - Ok(Some(url_to_file_path_string(&url)?)) + Ok(Some(url_or_path_to_string(url)?)) } else { Ok(None) } @@ -738,11 +732,6 @@ pub fn op_require_break_on_next_statement(state: Rc>) { inspector.wait_for_session_and_break_on_next_statement() } -fn url_to_file_path_string(url: &Url) -> Result { - let file_path = url_to_file_path(url)?; - Ok(file_path.to_string_lossy().into_owned()) -} - #[op2(fast)] pub fn op_require_can_parse_as_esm( scope: &mut v8::HandleScope, @@ -768,3 +757,13 @@ pub fn op_require_can_parse_as_esm( let mut source = v8::script_compiler::Source::new(source, Some(&origin)); v8::script_compiler::compile_module(scope, &mut source).is_some() } + +fn url_or_path_to_string( + url: UrlOrPath, +) -> Result { + if url.is_file() { + Ok(url.into_path()?.to_string_lossy().to_string()) + } else { + Ok(url.to_string_lossy().to_string()) + } +} diff --git a/resolvers/deno/cjs.rs b/resolvers/deno/cjs.rs index f3347fc9a0..76bc08e2bc 100644 --- a/resolvers/deno/cjs.rs +++ b/resolvers/deno/cjs.rs @@ -267,8 +267,11 @@ impl specifier: &Url, ) -> Result { if self.in_npm_pkg_checker.in_npm_package(specifier) { + let Ok(path) = deno_path_util::url_to_file_path(specifier) else { + return Ok(ResolutionMode::Require); + }; if let Some(pkg_json) = - self.pkg_json_resolver.get_closest_package_json(specifier)? + self.pkg_json_resolver.get_closest_package_json(&path)? { let is_file_location_cjs = pkg_json.typ != "module"; Ok(if is_file_location_cjs { @@ -280,8 +283,11 @@ impl Ok(ResolutionMode::Require) } } else if self.mode != IsCjsResolutionMode::Disabled { + let Ok(path) = deno_path_util::url_to_file_path(specifier) else { + return Ok(ResolutionMode::Import); + }; if let Some(pkg_json) = - self.pkg_json_resolver.get_closest_package_json(specifier)? + self.pkg_json_resolver.get_closest_package_json(&path)? { let is_cjs_type = pkg_json.typ == "commonjs" || self.mode == IsCjsResolutionMode::ImplicitTypeCommonJs diff --git a/resolvers/deno/factory.rs b/resolvers/deno/factory.rs index 7edb08991d..95db8415d4 100644 --- a/resolvers/deno/factory.rs +++ b/resolvers/deno/factory.rs @@ -639,7 +639,6 @@ impl< options: ResolverFactoryOptions, ) -> Self { Self { - options, deno_resolver: Default::default(), in_npm_package_checker: Default::default(), node_resolver: Default::default(), @@ -650,6 +649,7 @@ impl< sloppy_imports_resolver: Default::default(), workspace_factory, workspace_resolver: Default::default(), + options, } } diff --git a/resolvers/deno/lib.rs b/resolvers/deno/lib.rs index 49f741f95d..0ac073821c 100644 --- a/resolvers/deno/lib.rs +++ b/resolvers/deno/lib.rs @@ -92,6 +92,9 @@ pub enum DenoResolveErrorKind { PackageSubpathResolve(#[from] PackageSubpathResolveError), #[class(inherit)] #[error(transparent)] + PathToUrl(#[from] deno_path_util::PathToUrlError), + #[class(inherit)] + #[error(transparent)] ResolvePkgFolderFromDenoReq(#[from] ResolvePkgFolderFromDenoReqError), #[class(inherit)] #[error(transparent)] @@ -252,10 +255,12 @@ impl< { return node_resolver .resolve(raw_specifier, referrer, resolution_mode, resolution_kind) - .map(|res| DenoResolution { - url: res.into_url(), - found_package_json_dep, - maybe_diagnostic, + .and_then(|res| { + Ok(DenoResolution { + url: res.into_url()?, + found_package_json_dep, + maybe_diagnostic, + }) }) .map_err(|e| e.into()); } @@ -318,7 +323,8 @@ impl< resolution_mode, resolution_kind, ) - .map_err(|e| e.into()), + .map_err(DenoResolveError::from) + .and_then(|r| Ok(r.into_url()?)), MappedResolution::PackageJson { dep_result, alias, @@ -372,7 +378,8 @@ impl< .map_err(|e| { DenoResolveErrorKind::PackageSubpathResolve(e).into_box() }) - }), + }) + .and_then(|r| Ok(r.into_url()?)), }) } }, @@ -425,12 +432,14 @@ impl< resolution_mode, resolution_kind, ) - .map(|url| DenoResolution { - url, - maybe_diagnostic, - found_package_json_dep, - }) - .map_err(|e| e.into()); + .map_err(DenoResolveError::from) + .and_then(|url_or_path| { + Ok(DenoResolution { + url: url_or_path.into_url()?, + maybe_diagnostic, + found_package_json_dep, + }) + }); } // do npm resolution for byonm @@ -442,11 +451,6 @@ impl< resolution_mode, resolution_kind, ) - .map(|url| DenoResolution { - url, - maybe_diagnostic, - found_package_json_dep, - }) .map_err(|err| { match err.into_kind() { ResolveReqWithSubPathErrorKind::MissingPackageNodeModulesFolder( @@ -459,7 +463,12 @@ impl< err.into() } } - }); + }) + .and_then(|url_or_path| Ok(DenoResolution { + url: url_or_path.into_url()?, + maybe_diagnostic, + found_package_json_dep, + })); } } @@ -491,9 +500,9 @@ impl< })?; if let Some(res) = maybe_resolution { match res { - NodeResolution::Module(url) => { + NodeResolution::Module(ref _url) => { return Ok(DenoResolution { - url, + url: res.into_url()?, maybe_diagnostic, found_package_json_dep, }) diff --git a/resolvers/deno/npm/byonm.rs b/resolvers/deno/npm/byonm.rs index 4f72692f8b..a71309c7c3 100644 --- a/resolvers/deno/npm/byonm.rs +++ b/resolvers/deno/npm/byonm.rs @@ -18,6 +18,7 @@ use node_resolver::errors::PackageNotFoundError; use node_resolver::InNpmPackageChecker; use node_resolver::NpmPackageFolderResolver; use node_resolver::PackageJsonResolverRc; +use node_resolver::UrlOrPathRef; use sys_traits::FsCanonicalize; use sys_traits::FsDirEntry; use sys_traits::FsMetadata; @@ -141,7 +142,7 @@ impl ) -> std::io::Result> { for ancestor in start_dir.ancestors() { let node_modules_folder = ancestor.join("node_modules"); - let sub_dir = join_package_name(&node_modules_folder, alias); + let sub_dir = join_package_name(Cow::Owned(node_modules_folder), alias); if sys.fs_is_dir_no_err(&sub_dir) { return Ok(Some( deno_path_util::fs::canonicalize_path_maybe_not_exists( @@ -368,7 +369,7 @@ impl best_version.map(|(_version, entry_name)| { join_package_name( - &node_modules_deno_dir.join(entry_name).join("node_modules"), + Cow::Owned(node_modules_deno_dir.join(entry_name).join("node_modules")), &req.name, ) }) @@ -381,14 +382,14 @@ impl fn resolve_package_folder_from_package( &self, name: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { fn inner( sys: &TSys, name: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { - let maybe_referrer_file = url_to_file_path(referrer).ok(); + let maybe_referrer_file = referrer.path().ok(); let maybe_start_folder = maybe_referrer_file.as_ref().and_then(|f| f.parent()); if let Some(start_folder) = maybe_start_folder { @@ -400,7 +401,7 @@ impl Cow::Owned(current_folder.join("node_modules")) }; - let sub_dir = join_package_name(&node_modules_folder, name); + let sub_dir = join_package_name(node_modules_folder, name); if sys.fs_is_dir_no_err(&sub_dir) { return Ok(sub_dir); } @@ -410,7 +411,7 @@ impl Err( PackageNotFoundError { package_name: name.to_string(), - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: None, } .into(), @@ -421,7 +422,7 @@ impl self.sys.fs_canonicalize(&path).map_err(|err| { PackageFolderResolveIoError { package_name: name.to_string(), - referrer: referrer.clone(), + referrer: referrer.display(), source: err, } .into() @@ -442,11 +443,15 @@ impl InNpmPackageChecker for ByonmInNpmPackageChecker { } } -fn join_package_name(path: &Path, package_name: &str) -> PathBuf { - let mut path = path.to_path_buf(); +fn join_package_name(mut path: Cow, package_name: &str) -> PathBuf { // ensure backslashes are used on windows for part in package_name.split('/') { - path = path.join(part); + match path { + Cow::Borrowed(inner) => path = Cow::Owned(inner.join(part)), + Cow::Owned(ref mut path) => { + path.push(part); + } + } } - path + path.into_owned() } diff --git a/resolvers/deno/npm/managed/common.rs b/resolvers/deno/npm/managed/common.rs index a92fe226dd..74bd807181 100644 --- a/resolvers/deno/npm/managed/common.rs +++ b/resolvers/deno/npm/managed/common.rs @@ -6,6 +6,7 @@ use std::path::PathBuf; use deno_npm::NpmPackageCacheFolderId; use deno_npm::NpmPackageId; use node_resolver::NpmPackageFolderResolver; +use node_resolver::UrlOrPathRef; use sys_traits::FsCanonicalize; use sys_traits::FsMetadata; use url::Url; @@ -60,7 +61,7 @@ impl NpmPackageFolderResolver fn resolve_package_folder_from_package( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { match self { NpmPackageFsResolver::Local(r) => { diff --git a/resolvers/deno/npm/managed/global.rs b/resolvers/deno/npm/managed/global.rs index 4518108f5a..b365d33b9e 100644 --- a/resolvers/deno/npm/managed/global.rs +++ b/resolvers/deno/npm/managed/global.rs @@ -13,6 +13,7 @@ use node_resolver::errors::PackageFolderResolveError; use node_resolver::errors::PackageNotFoundError; use node_resolver::errors::ReferrerNotFoundError; use node_resolver::NpmPackageFolderResolver; +use node_resolver::UrlOrPathRef; use url::Url; use super::resolution::NpmResolutionCellRc; @@ -83,15 +84,15 @@ impl NpmPackageFolderResolver for GlobalNpmPackageResolver { fn resolve_package_folder_from_package( &self, name: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { use deno_npm::resolution::PackageNotFoundFromReferrerError; - let Some(referrer_cache_folder_id) = - self.resolve_package_cache_folder_id_from_specifier_inner(referrer) + let Some(referrer_cache_folder_id) = self + .resolve_package_cache_folder_id_from_specifier_inner(referrer.url()?) else { return Err( ReferrerNotFoundError { - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: None, } .into(), @@ -106,7 +107,7 @@ impl NpmPackageFolderResolver for GlobalNpmPackageResolver { None => Err( PackageNotFoundError { package_name: name.to_string(), - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: Some(format!( "{} -> {}", referrer_cache_folder_id, @@ -119,7 +120,7 @@ impl NpmPackageFolderResolver for GlobalNpmPackageResolver { Err(err) => match *err { PackageNotFoundFromReferrerError::Referrer(cache_folder_id) => Err( ReferrerNotFoundError { - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: Some(cache_folder_id.to_string()), } .into(), @@ -130,7 +131,7 @@ impl NpmPackageFolderResolver for GlobalNpmPackageResolver { } => Err( PackageNotFoundError { package_name: name, - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: Some(cache_folder_id_referrer.to_string()), } .into(), diff --git a/resolvers/deno/npm/managed/local.rs b/resolvers/deno/npm/managed/local.rs index e453101df4..6caab5a8d3 100644 --- a/resolvers/deno/npm/managed/local.rs +++ b/resolvers/deno/npm/managed/local.rs @@ -15,6 +15,7 @@ use node_resolver::errors::PackageFolderResolveIoError; use node_resolver::errors::PackageNotFoundError; use node_resolver::errors::ReferrerNotFoundError; use node_resolver::NpmPackageFolderResolver; +use node_resolver::UrlOrPathRef; use sys_traits::FsCanonicalize; use sys_traits::FsMetadata; use url::Url; @@ -149,19 +150,19 @@ impl NpmPackageFolderResolver fn resolve_package_folder_from_package( &self, name: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { let maybe_local_path = self - .resolve_folder_for_specifier(referrer) + .resolve_folder_for_specifier(referrer.url()?) .map_err(|err| PackageFolderResolveIoError { - package_name: name.to_string(), - referrer: referrer.clone(), - source: err, - })?; + package_name: name.to_string(), + referrer: referrer.display(), + source: err, + })?; let Some(local_path) = maybe_local_path else { return Err( ReferrerNotFoundError { - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: None, } .into(), @@ -182,7 +183,7 @@ impl NpmPackageFolderResolver return Ok(self.sys.fs_canonicalize(&sub_dir).map_err(|err| { PackageFolderResolveIoError { package_name: name.to_string(), - referrer: referrer.clone(), + referrer: referrer.display(), source: err, } })?); @@ -196,7 +197,7 @@ impl NpmPackageFolderResolver Err( PackageNotFoundError { package_name: name.to_string(), - referrer: referrer.clone(), + referrer: referrer.display(), referrer_extra: None, } .into(), diff --git a/resolvers/deno/npm/managed/mod.rs b/resolvers/deno/npm/managed/mod.rs index 9065db13cd..c74ce470b5 100644 --- a/resolvers/deno/npm/managed/mod.rs +++ b/resolvers/deno/npm/managed/mod.rs @@ -19,6 +19,7 @@ use deno_semver::package::PackageNv; use deno_semver::package::PackageReq; use node_resolver::InNpmPackageChecker; use node_resolver::NpmPackageFolderResolver; +use node_resolver::UrlOrPathRef; use sys_traits::FsCanonicalize; use sys_traits::FsMetadata; use url::Url; @@ -242,7 +243,7 @@ impl NpmPackageFolderResolver fn resolve_package_folder_from_package( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { let path = self .fs_resolver @@ -250,7 +251,7 @@ impl NpmPackageFolderResolver log::debug!( "Resolved {} from {} to {}", specifier, - referrer, + referrer.display(), path.display() ); Ok(path) diff --git a/resolvers/deno/npm/mod.rs b/resolvers/deno/npm/mod.rs index 6c6c29d63f..e4cf4704ad 100644 --- a/resolvers/deno/npm/mod.rs +++ b/resolvers/deno/npm/mod.rs @@ -22,6 +22,8 @@ use node_resolver::NodeResolutionKind; use node_resolver::NodeResolverRc; use node_resolver::NpmPackageFolderResolver; use node_resolver::ResolutionMode; +use node_resolver::UrlOrPath; +use node_resolver::UrlOrPathRef; use sys_traits::FsCanonicalize; use sys_traits::FsMetadata; use sys_traits::FsRead; @@ -234,7 +236,7 @@ impl fn resolve_package_folder_from_package( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { match self { NpmResolver::Byonm(byonm_resolver) => { @@ -331,7 +333,7 @@ impl< referrer: &Url, resolution_mode: ResolutionMode, resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { self.resolve_req_with_sub_path( req_ref.req(), req_ref.sub_path(), @@ -348,7 +350,7 @@ impl< referrer: &Url, resolution_mode: ResolutionMode, resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let package_folder = self .npm_resolver .resolve_pkg_folder_from_deno_module_req(req, referrer)?; @@ -398,6 +400,8 @@ impl< | NodeResolveErrorKind::PackageImportsResolve(_) | NodeResolveErrorKind::UnsupportedEsmUrlScheme(_) | NodeResolveErrorKind::DataUrlReferrer(_) + | NodeResolveErrorKind::PathToUrl(_) + | NodeResolveErrorKind::UrlToFilePath(_) | NodeResolveErrorKind::TypesNotFound(_) | NodeResolveErrorKind::FinalizeResolution(_) => Err( ResolveIfForNpmPackageErrorKind::NodeResolve(err.into()).into_box(), @@ -405,6 +409,12 @@ impl< NodeResolveErrorKind::PackageResolve(err) => { let err = err.into_kind(); match err { + PackageResolveErrorKind::UrlToFilePath(err) => Err( + ResolveIfForNpmPackageErrorKind::NodeResolve( + NodeResolveErrorKind::UrlToFilePath(err).into_box(), + ) + .into_box(), + ), PackageResolveErrorKind::ClosestPkgJson(_) | PackageResolveErrorKind::InvalidModuleSpecifier(_) | PackageResolveErrorKind::ExportsResolve(_) @@ -416,6 +426,12 @@ impl< ), PackageResolveErrorKind::PackageFolderResolve(err) => { match err.as_kind() { + PackageFolderResolveErrorKind::PathToUrl(err) => Err( + ResolveIfForNpmPackageErrorKind::NodeResolve( + NodeResolveErrorKind::PathToUrl(err.clone()).into_box(), + ) + .into_box(), + ), PackageFolderResolveErrorKind::Io( PackageFolderResolveIoError { package_name, .. }, ) diff --git a/resolvers/node/Cargo.toml b/resolvers/node/Cargo.toml index f6bfcf5fd2..fd0d8a8f0b 100644 --- a/resolvers/node/Cargo.toml +++ b/resolvers/node/Cargo.toml @@ -20,6 +20,7 @@ sync = ["deno_package_json/sync"] anyhow.workspace = true async-trait.workspace = true boxed_error.workspace = true +dashmap.workspace = true deno_error.workspace = true deno_package_json.workspace = true deno_path_util.workspace = true diff --git a/resolvers/node/analyze.rs b/resolvers/node/analyze.rs index b5fc224037..dd1e7dbc41 100644 --- a/resolvers/node/analyze.rs +++ b/resolvers/node/analyze.rs @@ -7,7 +7,6 @@ use std::path::Path; use std::path::PathBuf; use deno_error::JsErrorBox; -use deno_path_util::url_from_file_path; use deno_path_util::url_to_file_path; use futures::future::LocalBoxFuture; use futures::stream::FuturesUnordered; @@ -29,6 +28,8 @@ use crate::NpmPackageFolderResolver; use crate::PackageJsonResolverRc; use crate::PathClean; use crate::ResolutionMode; +use crate::UrlOrPath; +use crate::UrlOrPathRef; #[derive(Debug, Clone)] pub enum CjsAnalysis<'a> { @@ -253,14 +254,21 @@ impl< errors: &mut Vec| { // 1. Resolve the re-exports and start a future to analyze each one for reexport in reexports { - let result = self.resolve( - &reexport, - &referrer, - // FIXME(bartlomieju): check if these conditions are okay, probably - // should be `deno-require`, because `deno` is already used in `esm_resolver.rs` - &["deno", "node", "require", "default"], - NodeResolutionKind::Execution, - ); + let result = self + .resolve( + &reexport, + &referrer, + // FIXME(bartlomieju): check if these conditions are okay, probably + // should be `deno-require`, because `deno` is already used in `esm_resolver.rs` + &["deno", "node", "require", "default"], + NodeResolutionKind::Execution, + ) + .and_then(|value| { + value + .map(|url_or_path| url_or_path.into_url()) + .transpose() + .map_err(JsErrorBox::from_err) + }); let reexport_specifier = match result { Ok(Some(specifier)) => specifier, Ok(None) => continue, @@ -355,18 +363,18 @@ impl< referrer: &Url, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result, JsErrorBox> { + ) -> Result, JsErrorBox> { if specifier.starts_with('/') { todo!(); } - let referrer_path = url_to_file_path(referrer).unwrap(); + let referrer = UrlOrPathRef::from_url(referrer); + let referrer_path = referrer.path().unwrap(); if specifier.starts_with("./") || specifier.starts_with("../") { if let Some(parent) = referrer_path.parent() { return self - .file_extension_probe(parent.join(specifier), &referrer_path) - .and_then(|p| url_from_file_path(&p).map_err(JsErrorBox::from_err)) - .map(Some); + .file_extension_probe(parent.join(specifier), referrer_path) + .map(|p| Some(UrlOrPath::Path(p))); } else { todo!(); } @@ -376,20 +384,21 @@ impl< let (package_specifier, package_subpath) = parse_specifier(specifier).unwrap(); - let module_dir = match self - .npm_resolver - .resolve_package_folder_from_package(package_specifier.as_str(), referrer) - { - Err(err) - if matches!( - err.as_kind(), - crate::errors::PackageFolderResolveErrorKind::PackageNotFound(..) - ) => - { - return Ok(None); - } - other => other.map_err(JsErrorBox::from_err)?, - }; + let module_dir = + match self.npm_resolver.resolve_package_folder_from_package( + package_specifier.as_str(), + &referrer, + ) { + Err(err) + if matches!( + err.as_kind(), + crate::errors::PackageFolderResolveErrorKind::PackageNotFound(..) + ) => + { + return Ok(None); + } + other => other.map_err(JsErrorBox::from_err)?, + }; let package_json_path = module_dir.join("package.json"); let maybe_package_json = self @@ -405,7 +414,7 @@ impl< &package_json_path, &package_subpath, exports, - Some(referrer), + Some(&referrer), ResolutionMode::Import, conditions, resolution_kind, @@ -429,39 +438,26 @@ impl< if let Some(main) = package_json.main(deno_package_json::NodeModuleKind::Cjs) { - return Ok(Some( - url_from_file_path(&d.join(main).clean()) - .map_err(JsErrorBox::from_err)?, - )); + return Ok(Some(UrlOrPath::Path(d.join(main).clean()))); } } - return Ok(Some( - url_from_file_path(&d.join("index.js").clean()) - .map_err(JsErrorBox::from_err)?, - )); + return Ok(Some(UrlOrPath::Path(d.join("index.js").clean()))); } return self - .file_extension_probe(d, &referrer_path) - .and_then(|p| url_from_file_path(&p).map_err(JsErrorBox::from_err)) - .map(Some); + .file_extension_probe(d, referrer_path) + .map(|p| Some(UrlOrPath::Path(p))); } else if let Some(main) = package_json.main(deno_package_json::NodeModuleKind::Cjs) { - return Ok(Some( - url_from_file_path(&module_dir.join(main).clean()) - .map_err(JsErrorBox::from_err)?, - )); + return Ok(Some(UrlOrPath::Path(module_dir.join(main).clean()))); } else { - return Ok(Some( - url_from_file_path(&module_dir.join("index.js").clean()) - .map_err(JsErrorBox::from_err)?, - )); + return Ok(Some(UrlOrPath::Path(module_dir.join("index.js").clean()))); } } // as a fallback, attempt to resolve it via the ancestor directories - let mut last = referrer_path.as_path(); + let mut last = referrer_path; while let Some(parent) = last.parent() { if !self.in_npm_pkg_checker.in_npm_package_at_dir_path(parent) { break; @@ -471,15 +467,13 @@ impl< } else { parent.join("node_modules").join(specifier) }; - if let Ok(path) = self.file_extension_probe(path, &referrer_path) { - return Ok(Some( - url_from_file_path(&path).map_err(JsErrorBox::from_err)?, - )); + if let Ok(path) = self.file_extension_probe(path, referrer_path) { + return Ok(Some(UrlOrPath::Path(path))); } last = parent; } - Err(not_found(specifier, &referrer_path)) + Err(not_found(specifier, referrer_path)) } fn file_extension_probe( diff --git a/resolvers/node/errors.rs b/resolvers/node/errors.rs index 1b4ce460d1..85ab5ca444 100644 --- a/resolvers/node/errors.rs +++ b/resolvers/node/errors.rs @@ -6,9 +6,11 @@ use std::path::PathBuf; use boxed_error::Boxed; use deno_error::JsError; +use deno_path_util::UrlToFilePathError; use thiserror::Error; use url::Url; +use crate::path::UrlOrPath; use crate::NodeResolutionKind; use crate::ResolutionMode; @@ -24,6 +26,7 @@ pub enum NodeJsErrorCode { ERR_UNKNOWN_FILE_EXTENSION, ERR_UNSUPPORTED_DIR_IMPORT, ERR_UNSUPPORTED_ESM_URL_SCHEME, + ERR_INVALID_FILE_URL_PATH, /// Deno specific since Node doesn't support TypeScript. ERR_TYPES_NOT_FOUND, } @@ -48,6 +51,7 @@ impl NodeJsErrorCode { ERR_UNSUPPORTED_DIR_IMPORT => "ERR_UNSUPPORTED_DIR_IMPORT", ERR_UNSUPPORTED_ESM_URL_SCHEME => "ERR_UNSUPPORTED_ESM_URL_SCHEME", ERR_TYPES_NOT_FOUND => "ERR_TYPES_NOT_FOUND", + ERR_INVALID_FILE_URL_PATH => "ERR_INVALID_FILE_URL_PATH", } } } @@ -109,7 +113,7 @@ impl NodeJsErrorCoded for LegacyResolveError { #[class(generic)] pub struct PackageNotFoundError { pub package_name: String, - pub referrer: Url, + pub referrer: UrlOrPath, /// Extra information about the referrer. pub referrer_extra: Option, } @@ -128,7 +132,7 @@ impl NodeJsErrorCoded for PackageNotFoundError { )] #[class(generic)] pub struct ReferrerNotFoundError { - pub referrer: Url, + pub referrer: UrlOrPath, /// Extra information about the referrer. pub referrer_extra: Option, } @@ -144,7 +148,7 @@ impl NodeJsErrorCoded for ReferrerNotFoundError { #[error("Failed resolving '{package_name}' from referrer '{referrer}'.")] pub struct PackageFolderResolveIoError { pub package_name: String, - pub referrer: Url, + pub referrer: UrlOrPath, #[source] #[inherit] pub source: std::io::Error, @@ -162,6 +166,9 @@ impl NodeJsErrorCoded for PackageFolderResolveError { PackageFolderResolveErrorKind::PackageNotFound(e) => e.code(), PackageFolderResolveErrorKind::ReferrerNotFound(e) => e.code(), PackageFolderResolveErrorKind::Io(e) => e.code(), + PackageFolderResolveErrorKind::PathToUrl(_) => { + NodeJsErrorCode::ERR_INVALID_FILE_URL_PATH + } } } } @@ -180,6 +187,9 @@ pub enum PackageFolderResolveErrorKind { #[class(inherit)] #[error(transparent)] Io(#[from] PackageFolderResolveIoError), + #[class(inherit)] + #[error(transparent)] + PathToUrl(#[from] deno_path_util::PathToUrlError), } impl NodeJsErrorCoded for PackageSubpathResolveError { @@ -232,7 +242,7 @@ pub enum PackageSubpathResolveErrorKind { pub struct PackageTargetNotFoundError { pub pkg_json_path: PathBuf, pub target: String, - pub maybe_referrer: Option, + pub maybe_referrer: Option, pub resolution_mode: ResolutionMode, pub resolution_kind: NodeResolutionKind, } @@ -251,6 +261,9 @@ impl NodeJsErrorCoded for PackageTargetResolveError { PackageTargetResolveErrorKind::InvalidModuleSpecifier(e) => e.code(), PackageTargetResolveErrorKind::PackageResolve(e) => e.code(), PackageTargetResolveErrorKind::TypesNotFound(e) => e.code(), + PackageTargetResolveErrorKind::UrlToFilePath(_) => { + NodeJsErrorCode::ERR_INVALID_FILE_URL_PATH + } } } } @@ -275,6 +288,9 @@ pub enum PackageTargetResolveErrorKind { #[class(inherit)] #[error(transparent)] TypesNotFound(#[from] TypesNotFoundError), + #[class(inherit)] + #[error(transparent)] + UrlToFilePath(#[from] deno_path_util::UrlToFilePathError), } impl NodeJsErrorCoded for PackageExportsResolveError { @@ -311,8 +327,8 @@ pub struct TypesNotFoundError(pub Box); #[derive(Debug)] pub struct TypesNotFoundErrorData { - pub code_specifier: Url, - pub maybe_referrer: Option, + pub code_specifier: UrlOrPath, + pub maybe_referrer: Option, } impl NodeJsErrorCoded for TypesNotFoundError { @@ -369,7 +385,7 @@ pub enum ClosestPkgJsonErrorKind { pub struct PackageImportNotDefinedError { pub name: String, pub package_json_path: Option, - pub maybe_referrer: Option, + pub maybe_referrer: Option, } impl NodeJsErrorCoded for PackageImportNotDefinedError { @@ -416,6 +432,9 @@ impl NodeJsErrorCoded for PackageResolveError { PackageResolveErrorKind::PackageFolderResolve(e) => e.code(), PackageResolveErrorKind::ExportsResolve(e) => e.code(), PackageResolveErrorKind::SubpathResolve(e) => e.code(), + PackageResolveErrorKind::UrlToFilePath(_) => { + NodeJsErrorCode::ERR_INVALID_FILE_URL_PATH + } } } } @@ -440,6 +459,9 @@ pub enum PackageResolveErrorKind { #[class(inherit)] #[error(transparent)] SubpathResolve(#[from] PackageSubpathResolveError), + #[class(inherit)] + #[error(transparent)] + UrlToFilePath(#[from] UrlToFilePathError), } #[derive(Debug, Error, JsError)] @@ -470,6 +492,12 @@ pub enum NodeResolveErrorKind { RelativeJoin(#[from] NodeResolveRelativeJoinError), #[class(inherit)] #[error(transparent)] + PathToUrl(#[from] deno_path_util::PathToUrlError), + #[class(inherit)] + #[error(transparent)] + UrlToFilePath(#[from] deno_path_util::UrlToFilePathError), + #[class(inherit)] + #[error(transparent)] PackageImportsResolve(#[from] PackageImportsResolveError), #[class(inherit)] #[error(transparent)] @@ -502,6 +530,9 @@ pub enum FinalizeResolutionErrorKind { #[class(inherit)] #[error(transparent)] UnsupportedDirImport(#[from] UnsupportedDirImportError), + #[class(inherit)] + #[error(transparent)] + UrlToFilePath(#[from] deno_path_util::UrlToFilePathError), } impl NodeJsErrorCoded for FinalizeResolutionError { @@ -510,6 +541,9 @@ impl NodeJsErrorCoded for FinalizeResolutionError { FinalizeResolutionErrorKind::InvalidModuleSpecifierError(e) => e.code(), FinalizeResolutionErrorKind::ModuleNotFound(e) => e.code(), FinalizeResolutionErrorKind::UnsupportedDirImport(e) => e.code(), + FinalizeResolutionErrorKind::UrlToFilePath(_) => { + NodeJsErrorCode::ERR_INVALID_FILE_URL_PATH + } } } } @@ -524,8 +558,8 @@ impl NodeJsErrorCoded for FinalizeResolutionError { maybe_referrer.as_ref().map(|referrer| format!(" imported from '{}'", referrer)).unwrap_or_default() )] pub struct ModuleNotFoundError { - pub specifier: Url, - pub maybe_referrer: Option, + pub specifier: UrlOrPath, + pub maybe_referrer: Option, pub typ: &'static str, } @@ -544,8 +578,8 @@ impl NodeJsErrorCoded for ModuleNotFoundError { maybe_referrer.as_ref().map(|referrer| format!(" imported from '{}'", referrer)).unwrap_or_default(), )] pub struct UnsupportedDirImportError { - pub dir_url: Url, - pub maybe_referrer: Option, + pub dir_url: UrlOrPath, + pub maybe_referrer: Option, } impl NodeJsErrorCoded for UnsupportedDirImportError { @@ -561,7 +595,7 @@ pub struct InvalidPackageTargetError { pub sub_path: String, pub target: String, pub is_import: bool, - pub maybe_referrer: Option, + pub maybe_referrer: Option, } impl std::error::Error for InvalidPackageTargetError {} @@ -616,7 +650,7 @@ impl NodeJsErrorCoded for InvalidPackageTargetError { pub struct PackagePathNotExportedError { pub pkg_json_path: PathBuf, pub subpath: String, - pub maybe_referrer: Option, + pub maybe_referrer: Option, pub resolution_kind: NodeResolutionKind, } diff --git a/resolvers/node/lib.rs b/resolvers/node/lib.rs index 42053f73cc..a5516f449a 100644 --- a/resolvers/node/lib.rs +++ b/resolvers/node/lib.rs @@ -24,6 +24,8 @@ pub use package_json::PackageJsonResolver; pub use package_json::PackageJsonResolverRc; pub use package_json::PackageJsonThreadLocalCache; pub use path::PathClean; +pub use path::UrlOrPath; +pub use path::UrlOrPathRef; pub use resolution::parse_npm_pkg_name; pub use resolution::resolve_specifier_into_node_modules; pub use resolution::ConditionsFromResolutionMode; diff --git a/resolvers/node/npm.rs b/resolvers/node/npm.rs index bb3de2a7f5..89426a71bf 100644 --- a/resolvers/node/npm.rs +++ b/resolvers/node/npm.rs @@ -9,13 +9,14 @@ use url::Url; use crate::errors; use crate::path::PathClean; +use crate::path::UrlOrPathRef; pub trait NpmPackageFolderResolver { /// Resolves an npm package folder path from the specified referrer. fn resolve_package_folder_from_package( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result; } diff --git a/resolvers/node/package_json.rs b/resolvers/node/package_json.rs index ec3c8addbf..cd98aa4e12 100644 --- a/resolvers/node/package_json.rs +++ b/resolvers/node/package_json.rs @@ -9,7 +9,6 @@ use std::path::PathBuf; use deno_package_json::PackageJson; use deno_package_json::PackageJsonRc; use sys_traits::FsRead; -use url::Url; use crate::errors::ClosestPkgJsonError; use crate::errors::PackageJsonLoadError; @@ -51,17 +50,17 @@ pub struct PackageJsonThreadLocalCache; impl PackageJsonThreadLocalCache { pub fn clear() { - CACHE.with(|cache| cache.borrow_mut().clear()); + CACHE.with_borrow_mut(|cache| cache.clear()); } } impl deno_package_json::PackageJsonCache for PackageJsonThreadLocalCache { fn get(&self, path: &Path) -> Option { - CACHE.with(|cache| cache.borrow().get(path).cloned()) + CACHE.with_borrow(|cache| cache.get(path).cloned()) } fn set(&self, path: PathBuf, package_json: PackageJsonRc) { - CACHE.with(|cache| cache.borrow_mut().insert(path, package_json)); + CACHE.with_borrow_mut(|cache| cache.insert(path, package_json)); } } @@ -81,20 +80,12 @@ impl PackageJsonResolver { } pub fn get_closest_package_json( - &self, - url: &Url, - ) -> Result, ClosestPkgJsonError> { - let Ok(file_path) = deno_path_util::url_to_file_path(url) else { - return Ok(None); - }; - self.get_closest_package_json_from_file_path(&file_path) - } - - pub fn get_closest_package_json_from_file_path( &self, file_path: &Path, ) -> Result, ClosestPkgJsonError> { - let parent_dir = file_path.parent().unwrap(); + let Some(parent_dir) = file_path.parent() else { + return Ok(None); + }; for current_dir in parent_dir.ancestors() { let package_json_path = current_dir.join("package.json"); if let Some(pkg_json) = self.load_package_json(&package_json_path)? { diff --git a/resolvers/node/path.rs b/resolvers/node/path.rs index 525aeb36ef..88aba75e86 100644 --- a/resolvers/node/path.rs +++ b/resolvers/node/path.rs @@ -1,9 +1,125 @@ // Copyright 2018-2025 the Deno authors. MIT license. +use std::borrow::Cow; use std::path::Component; use std::path::Path; use std::path::PathBuf; +use url::Url; + +#[derive(Debug, Clone)] +pub enum UrlOrPath { + Url(Url), + Path(PathBuf), +} + +impl UrlOrPath { + pub fn is_file(&self) -> bool { + match self { + UrlOrPath::Url(url) => url.scheme() == "file", + UrlOrPath::Path(_) => true, + } + } + + pub fn is_node_url(&self) -> bool { + match self { + UrlOrPath::Url(url) => url.scheme() == "node", + UrlOrPath::Path(_) => false, + } + } + + pub fn into_path( + self, + ) -> Result { + match self { + UrlOrPath::Url(url) => deno_path_util::url_to_file_path(&url), + UrlOrPath::Path(path) => Ok(path), + } + } + + pub fn into_url(self) -> Result { + match self { + UrlOrPath::Url(url) => Ok(url), + UrlOrPath::Path(path) => deno_path_util::url_from_file_path(&path), + } + } + + pub fn to_string_lossy(&self) -> Cow { + match self { + UrlOrPath::Url(url) => Cow::Borrowed(url.as_str()), + UrlOrPath::Path(path) => path.to_string_lossy(), + } + } +} + +impl std::fmt::Display for UrlOrPath { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + UrlOrPath::Url(url) => url.fmt(f), + UrlOrPath::Path(path) => { + // prefer displaying a url + match deno_path_util::url_from_file_path(path) { + Ok(url) => url.fmt(f), + Err(_) => { + write!(f, "{}", path.display()) + } + } + } + } + } +} + +pub struct UrlOrPathRef<'a> { + url: once_cell::unsync::OnceCell>, + path: once_cell::unsync::OnceCell>, +} + +impl<'a> UrlOrPathRef<'a> { + pub fn from_path(path: &'a Path) -> Self { + Self { + url: Default::default(), + path: once_cell::unsync::OnceCell::with_value(Cow::Borrowed(path)), + } + } + + pub fn from_url(url: &'a Url) -> Self { + Self { + path: Default::default(), + url: once_cell::unsync::OnceCell::with_value(Cow::Borrowed(url)), + } + } + + pub fn url(&self) -> Result<&Url, deno_path_util::PathToUrlError> { + self + .url + .get_or_try_init(|| { + deno_path_util::url_from_file_path(self.path.get().unwrap()) + .map(Cow::Owned) + }) + .map(|v| v.as_ref()) + } + + pub fn path(&self) -> Result<&Path, deno_path_util::UrlToFilePathError> { + self + .path + .get_or_try_init(|| { + deno_path_util::url_to_file_path(self.url.get().unwrap()) + .map(Cow::Owned) + }) + .map(|v| v.as_ref()) + } + + pub fn display(&self) -> UrlOrPath { + // prefer url + if let Ok(url) = self.url() { + UrlOrPath::Url(url.clone()) + } else { + // this will always be set if url is None + UrlOrPath::Path(self.path.get().unwrap().to_path_buf()) + } + } +} + /// Extension to path_clean::PathClean pub trait PathClean { fn clean(&self) -> T; diff --git a/resolvers/node/resolution.rs b/resolvers/node/resolution.rs index c0df61bed5..273c630b27 100644 --- a/resolvers/node/resolution.rs +++ b/resolvers/node/resolution.rs @@ -8,7 +8,6 @@ use std::path::PathBuf; use anyhow::bail; use anyhow::Error as AnyError; use deno_package_json::PackageJson; -use deno_path_util::url_from_file_path; use serde_json::Map; use serde_json::Value; use sys_traits::FileType; @@ -46,6 +45,8 @@ use crate::errors::TypesNotFoundError; use crate::errors::TypesNotFoundErrorData; use crate::errors::UnsupportedDirImportError; use crate::errors::UnsupportedEsmUrlSchemeError; +use crate::path::UrlOrPath; +use crate::path::UrlOrPathRef; use crate::InNpmPackageChecker; use crate::IsBuiltInNodeModuleChecker; use crate::NpmPackageFolderResolver; @@ -115,21 +116,19 @@ impl NodeResolutionKind { #[derive(Debug)] pub enum NodeResolution { - Module(Url), + Module(UrlOrPath), BuiltIn(String), } impl NodeResolution { - pub fn into_url(self) -> Url { + pub fn into_url(self) -> Result { match self { - Self::Module(u) => u, - Self::BuiltIn(specifier) => { - if specifier.starts_with("node:") { - Url::parse(&specifier).unwrap() - } else { - Url::parse(&format!("node:{specifier}")).unwrap() - } - } + Self::Module(u) => Ok(u.into_url()?), + Self::BuiltIn(specifier) => Ok(if specifier.starts_with("node:") { + Url::parse(&specifier).unwrap() + } else { + Url::parse(&format!("node:{specifier}")).unwrap() + }), } } } @@ -220,7 +219,7 @@ impl< if let Ok(url) = Url::parse(specifier) { if url.scheme() == "data" { - return Ok(NodeResolution::Module(url)); + return Ok(NodeResolution::Module(UrlOrPath::Url(url))); } if let Some(module_name) = @@ -245,26 +244,27 @@ impl< let url = referrer .join(specifier) .map_err(|source| DataUrlReferrerError { source })?; - return Ok(NodeResolution::Module(url)); + return Ok(NodeResolution::Module(UrlOrPath::Url(url))); } } let conditions = self .conditions_from_resolution_mode .resolve(resolution_mode); + let referrer = UrlOrPathRef::from_url(referrer); let url = self.module_resolve( specifier, - referrer, + &referrer, resolution_mode, conditions, resolution_kind, )?; - let url = if resolution_kind.is_types() { - let file_path = to_file_path(&url); - self.path_to_declaration_url( - &file_path, - Some(referrer), + let url = if resolution_kind.is_types() && url.is_file() { + let file_path = url.into_path()?; + self.path_to_declaration_path( + file_path, + Some(&referrer), resolution_mode, conditions, )? @@ -272,8 +272,8 @@ impl< url }; - let url = self.finalize_resolution(url, Some(referrer))?; - let resolve_response = NodeResolution::Module(url); + let url_or_path = self.finalize_resolution(url, Some(&referrer))?; + let resolve_response = NodeResolution::Module(url_or_path); // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. Ok(resolve_response) @@ -282,23 +282,26 @@ impl< fn module_resolve( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { if should_be_treated_as_relative_or_absolute_path(specifier) { - Ok(node_join_url(referrer, specifier).map_err(|err| { - NodeResolveRelativeJoinError { - path: specifier.to_string(), - base: referrer.clone(), - source: err, - } - })?) + let referrer_url = referrer.url()?; + Ok(UrlOrPath::Url( + node_join_url(referrer_url, specifier).map_err(|err| { + NodeResolveRelativeJoinError { + path: specifier.to_string(), + base: referrer_url.clone(), + source: err, + } + })?, + )) } else if specifier.starts_with('#') { let pkg_config = self .pkg_json_resolver - .get_closest_package_json(referrer) + .get_closest_package_json(referrer.path()?) .map_err(PackageImportsResolveErrorKind::ClosestPkgJson) .map_err(|err| PackageImportsResolveError(Box::new(err)))?; Ok(self.package_imports_resolve( @@ -310,7 +313,7 @@ impl< resolution_kind, )?) } else if let Ok(resolved) = Url::parse(specifier) { - Ok(resolved) + Ok(UrlOrPath::Url(resolved)) } else { Ok(self.package_resolve( specifier, @@ -324,29 +327,37 @@ impl< fn finalize_resolution( &self, - resolved: Url, - maybe_referrer: Option<&Url>, - ) -> Result { + resolved: UrlOrPath, + maybe_referrer: Option<&UrlOrPathRef>, + ) -> Result { let encoded_sep_re = lazy_regex::regex!(r"%2F|%2C"); - if encoded_sep_re.is_match(resolved.path()) { + let text = match &resolved { + UrlOrPath::Url(url) => Cow::Borrowed(url.as_str()), + UrlOrPath::Path(path_buf) => path_buf.to_string_lossy(), + }; + if encoded_sep_re.is_match(&text) { return Err( errors::InvalidModuleSpecifierError { request: resolved.to_string(), reason: Cow::Borrowed( "must not include encoded \"/\" or \"\\\\\" characters", ), - maybe_referrer: maybe_referrer.map(to_file_path_string), + maybe_referrer: maybe_referrer.map(|r| match r.path() { + // in this case, prefer showing the path string + Ok(path) => path.display().to_string(), + Err(_) => r.display().to_string(), + }), } .into(), ); } - if resolved.scheme() == "node" { + if resolved.is_node_url() { return Ok(resolved); } - let path = to_file_path(&resolved); + let path = resolved.into_path()?; // TODO(bartlomieju): currently not supported // if (getOptionValue('--experimental-specifier-resolution') === 'node') { @@ -354,26 +365,27 @@ impl< // } let p_str = path.to_str().unwrap(); - let p = if p_str.ends_with('/') { - p_str[p_str.len() - 1..].to_string() + let path = if p_str.ends_with('/') { + // todo(dsherret): don't allocate a new string here + PathBuf::from(p_str[p_str.len() - 1..].to_string()) } else { - p_str.to_string() + path }; - let maybe_file_type = self.sys.fs_metadata(p).map(|m| m.file_type()); + let maybe_file_type = self.sys.fs_metadata(&path).map(|m| m.file_type()); match maybe_file_type { Ok(FileType::Dir) => Err( UnsupportedDirImportError { - dir_url: resolved.clone(), - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + dir_url: UrlOrPath::Path(path), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ), - Ok(FileType::File) => Ok(resolved), + Ok(FileType::File) => Ok(UrlOrPath::Path(path)), _ => Err( ModuleNotFoundError { - specifier: resolved, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + specifier: UrlOrPath::Path(path), + maybe_referrer: maybe_referrer.map(|r| r.display()), typ: "module", } .into(), @@ -388,16 +400,17 @@ impl< maybe_referrer: Option<&Url>, resolution_mode: ResolutionMode, resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { // todo(dsherret): don't allocate a string here (maybe use an // enum that says the subpath is not prefixed with a ./) let package_subpath = package_subpath .map(|s| format!("./{s}")) .unwrap_or_else(|| ".".to_string()); + let maybe_referrer = maybe_referrer.map(UrlOrPathRef::from_url); let resolved_url = self.resolve_package_dir_subpath( package_dir, &package_subpath, - maybe_referrer, + maybe_referrer.as_ref(), resolution_mode, self .conditions_from_resolution_mode @@ -441,7 +454,7 @@ impl< &self, package_folder: &Path, sub_path: Option<&str>, - ) -> Result { + ) -> Result { let pkg_json_path = package_folder.join("package.json"); let Some(package_json) = self.pkg_json_resolver.load_package_json(&pkg_json_path)? @@ -456,18 +469,16 @@ impl< message: err.to_string(), } })?; - let url = url_from_file_path(&package_folder.join(bin_entry)).unwrap(); - // TODO(bartlomieju): skipped checking errors for commonJS resolution and // "preserveSymlinksMain"/"preserveSymlinks" options. - Ok(url) + Ok(package_folder.join(bin_entry)) } /// Resolves an npm package folder path from the specified referrer. pub fn resolve_package_folder_from_package( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result { self .npm_pkg_folder_resolver @@ -475,13 +486,13 @@ impl< } /// Checks if the resolved file has a corresponding declaration file. - fn path_to_declaration_url( + fn path_to_declaration_path( &self, - path: &Path, - maybe_referrer: Option<&Url>, + path: PathBuf, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], - ) -> Result { + ) -> Result { fn probe_extensions( sys: &TSys, path: &Path, @@ -492,20 +503,20 @@ impl< let mut searched_for_d_cts = false; if lowercase_path.ends_with(".mjs") { let d_mts_path = with_known_extension(path, "d.mts"); - if sys.fs_exists_no_err(&d_mts_path) { + if sys.fs_is_file_no_err(&d_mts_path) { return Some(d_mts_path); } searched_for_d_mts = true; } else if lowercase_path.ends_with(".cjs") { let d_cts_path = with_known_extension(path, "d.cts"); - if sys.fs_exists_no_err(&d_cts_path) { + if sys.fs_is_file_no_err(&d_cts_path) { return Some(d_cts_path); } searched_for_d_cts = true; } let dts_path = with_known_extension(path, "d.ts"); - if sys.fs_exists_no_err(&dts_path) { + if sys.fs_is_file_no_err(&dts_path) { return Some(dts_path); } @@ -519,7 +530,7 @@ impl< _ => None, // already searched above }; if let Some(specific_dts_path) = specific_dts_path { - if sys.fs_exists_no_err(&specific_dts_path) { + if sys.fs_is_file_no_err(&specific_dts_path) { return Some(specific_dts_path); } } @@ -531,16 +542,16 @@ impl< || lowercase_path.ends_with(".d.cts") || lowercase_path.ends_with(".d.mts") { - return Ok(url_from_file_path(path).unwrap()); + return Ok(UrlOrPath::Path(path)); } if let Some(path) = - probe_extensions(&self.sys, path, &lowercase_path, resolution_mode) + probe_extensions(&self.sys, &path, &lowercase_path, resolution_mode) { - return Ok(url_from_file_path(&path).unwrap()); + return Ok(UrlOrPath::Path(path)); } - if self.sys.fs_is_dir_no_err(path) { + if self.sys.fs_is_dir_no_err(&path) { let resolution_result = self.resolve_package_dir_subpath( - path, + &path, /* sub path */ ".", maybe_referrer, resolution_mode, @@ -557,16 +568,16 @@ impl< &index_path.to_string_lossy().to_lowercase(), resolution_mode, ) { - return Ok(url_from_file_path(&path).unwrap()); + return Ok(UrlOrPath::Path(path)); } } // allow resolving .css files for types resolution if lowercase_path.ends_with(".css") { - return Ok(url_from_file_path(path).unwrap()); + return Ok(UrlOrPath::Path(path)); } Err(TypesNotFoundError(Box::new(TypesNotFoundErrorData { - code_specifier: url_from_file_path(path).unwrap(), - maybe_referrer: maybe_referrer.cloned(), + code_specifier: UrlOrPathRef::from_path(&path).display(), + maybe_referrer: maybe_referrer.map(|r| r.display()), }))) } @@ -574,12 +585,12 @@ impl< pub fn package_imports_resolve( &self, name: &str, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, referrer_pkg_json: Option<&PackageJson>, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { if name == "#" || name.starts_with("#/") || name.ends_with('/') { let reason = "is not a valid internal imports specifier name"; return Err( @@ -662,7 +673,7 @@ impl< PackageImportNotDefinedError { name: name.to_string(), package_json_path, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ) @@ -675,13 +686,13 @@ impl< subpath: &str, match_: &str, package_json_path: &Path, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, pattern: bool, internal: bool, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { if !subpath.is_empty() && !pattern && !target.ends_with('/') { return Err( InvalidPackageTargetError { @@ -689,7 +700,7 @@ impl< sub_path: match_.to_string(), target: target.to_string(), is_import: internal, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ); @@ -705,7 +716,7 @@ impl< if get_module_name_from_builtin_node_module_specifier(&url) .is_some() { - return Ok(url); + return Ok(UrlOrPath::Url(url)); } } Err(_) => { @@ -716,18 +727,17 @@ impl< } else { format!("{target}{subpath}") }; - let package_json_url = - url_from_file_path(package_json_path).unwrap(); let result = match self.package_resolve( &export_target, - &package_json_url, + &UrlOrPathRef::from_path(package_json_path), resolution_mode, conditions, resolution_kind, ) { Ok(url) => Ok(url), Err(err) => match err.code() { - NodeJsErrorCode::ERR_INVALID_MODULE_SPECIFIER + NodeJsErrorCode::ERR_INVALID_FILE_URL_PATH + | NodeJsErrorCode::ERR_INVALID_MODULE_SPECIFIER | NodeJsErrorCode::ERR_INVALID_PACKAGE_CONFIG | NodeJsErrorCode::ERR_INVALID_PACKAGE_TARGET | NodeJsErrorCode::ERR_PACKAGE_IMPORT_NOT_DEFINED @@ -743,7 +753,7 @@ impl< PackageTargetNotFoundError { pkg_json_path: package_json_path.to_path_buf(), target: export_target.to_string(), - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), resolution_mode, resolution_kind, }, @@ -760,7 +770,9 @@ impl< .is_built_in_node_module_checker .is_builtin_node_module(target) { - Ok(Url::parse(&format!("node:{}", target)).unwrap()) + Ok(UrlOrPath::Url( + Url::parse(&format!("node:{}", target)).unwrap(), + )) } else { Err(err) } @@ -775,7 +787,7 @@ impl< sub_path: match_.to_string(), target: target.to_string(), is_import: internal, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ); @@ -787,7 +799,7 @@ impl< sub_path: match_.to_string(), target: target.to_string(), is_import: internal, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ); @@ -801,13 +813,13 @@ impl< sub_path: match_.to_string(), target: target.to_string(), is_import: internal, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ); } if subpath.is_empty() { - return Ok(url_from_file_path(&resolved_path).unwrap()); + return Ok(UrlOrPath::Path(resolved_path)); } if invalid_segment_re.is_match(subpath) { let request = if pattern { @@ -829,11 +841,9 @@ impl< let resolved_path_str = resolved_path.to_string_lossy(); let replaced = pattern_re .replace(&resolved_path_str, |_caps: ®ex::Captures| subpath); - return Ok( - url_from_file_path(&PathBuf::from(replaced.to_string())).unwrap(), - ); + return Ok(UrlOrPath::Path(PathBuf::from(replaced.to_string()))); } - Ok(url_from_file_path(&resolved_path.join(subpath).clean()).unwrap()) + Ok(UrlOrPath::Path(resolved_path.join(subpath).clean())) } #[allow(clippy::too_many_arguments)] @@ -843,13 +853,13 @@ impl< target: &Value, subpath: &str, package_subpath: &str, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, pattern: bool, internal: bool, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result, PackageTargetResolveError> { + ) -> Result, PackageTargetResolveError> { let result = self.resolve_package_target_inner( package_json_path, target, @@ -899,15 +909,15 @@ impl< target: &Value, subpath: &str, package_subpath: &str, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, pattern: bool, internal: bool, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result, PackageTargetResolveError> { + ) -> Result, PackageTargetResolveError> { if let Some(target) = target.as_str() { - let url = self.resolve_package_target_string( + let url_or_path = self.resolve_package_target_string( target, subpath, package_subpath, @@ -919,16 +929,16 @@ impl< conditions, resolution_kind, )?; - if resolution_kind.is_types() && url.scheme() == "file" { - let path = deno_path_util::url_to_file_path(&url).unwrap(); - return Ok(Some(self.path_to_declaration_url( - &path, + if resolution_kind.is_types() && url_or_path.is_file() { + let path = url_or_path.into_path()?; + return Ok(Some(self.path_to_declaration_path( + path, maybe_referrer, resolution_mode, conditions, )?)); } else { - return Ok(Some(url)); + return Ok(Some(url_or_path)); } } else if let Some(target_arr) = target.as_array() { if target_arr.is_empty() { @@ -1013,7 +1023,7 @@ impl< sub_path: package_subpath.to_string(), target: target.to_string(), is_import: internal, - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ) @@ -1025,11 +1035,11 @@ impl< package_json_path: &Path, package_subpath: &str, package_exports: &Map, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { if let Some(target) = package_exports.get(package_subpath) { if package_subpath.find('*').is_none() && !package_subpath.ends_with('/') { @@ -1051,7 +1061,7 @@ impl< PackagePathNotExportedError { pkg_json_path: package_json_path.to_path_buf(), subpath: package_subpath.to_string(), - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), resolution_kind, } .into(), @@ -1116,7 +1126,7 @@ impl< PackagePathNotExportedError { pkg_json_path: package_json_path.to_path_buf(), subpath: package_subpath.to_string(), - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), resolution_kind, } .into(), @@ -1128,7 +1138,7 @@ impl< PackagePathNotExportedError { pkg_json_path: package_json_path.to_path_buf(), subpath: package_subpath.to_string(), - maybe_referrer: maybe_referrer.map(ToOwned::to_owned), + maybe_referrer: maybe_referrer.map(|r| r.display()), resolution_kind, } .into(), @@ -1138,16 +1148,17 @@ impl< pub(super) fn package_resolve( &self, specifier: &str, - referrer: &Url, + referrer: &UrlOrPathRef, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let (package_name, package_subpath, _is_scoped) = parse_npm_pkg_name(specifier, referrer)?; - if let Some(package_config) = - self.pkg_json_resolver.get_closest_package_json(referrer)? + if let Some(package_config) = self + .pkg_json_resolver + .get_closest_package_json(referrer.path()?)? { // ResolveSelf if package_config.name.as_deref() == Some(package_name) { @@ -1182,11 +1193,11 @@ impl< &self, package_name: &str, package_subpath: &str, - referrer: &Url, + referrer: &UrlOrPathRef, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let result = self.resolve_package_subpath_for_package_inner( package_name, package_subpath, @@ -1195,7 +1206,7 @@ impl< conditions, resolution_kind, ); - if resolution_kind.is_types() && !matches!(result, Ok(Url { .. })) { + if resolution_kind.is_types() && result.is_err() { // try to resolve with the @types package let package_name = types_package_name(package_name); if let Ok(result) = self.resolve_package_subpath_for_package_inner( @@ -1217,11 +1228,11 @@ impl< &self, package_name: &str, package_subpath: &str, - referrer: &Url, + referrer: &UrlOrPathRef, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let package_dir_path = self .npm_pkg_folder_resolver .resolve_package_folder_from_package(package_name, referrer)?; @@ -1257,11 +1268,11 @@ impl< &self, package_dir_path: &Path, package_subpath: &str, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let package_json_path = package_dir_path.join("package.json"); match self .pkg_json_resolver @@ -1295,11 +1306,11 @@ impl< &self, package_json: &PackageJson, package_subpath: &str, - referrer: Option<&Url>, + referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { if let Some(exports) = &package_json.exports { let result = self.package_exports_resolve( &package_json.path, @@ -1365,22 +1376,22 @@ impl< &self, directory: &Path, package_subpath: &str, - referrer: Option<&Url>, + referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { assert_ne!(package_subpath, "."); let file_path = directory.join(package_subpath); if resolution_kind.is_types() { - Ok(self.path_to_declaration_url( - &file_path, + Ok(self.path_to_declaration_path( + file_path, referrer, resolution_mode, conditions, )?) } else { - Ok(url_from_file_path(&file_path).unwrap()) + Ok(UrlOrPath::Path(file_path)) } } @@ -1388,18 +1399,20 @@ impl< &self, directory: &Path, package_subpath: &str, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { if package_subpath == "." { - self.legacy_index_resolve( - directory, - maybe_referrer, - resolution_mode, - resolution_kind, - ) + self + .legacy_index_resolve( + directory, + maybe_referrer, + resolution_mode, + resolution_kind, + ) + .map(UrlOrPath::Path) } else { self .resolve_subpath_exact( @@ -1417,11 +1430,11 @@ impl< pub(super) fn legacy_main_resolve( &self, package_json: &PackageJson, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, conditions: &[&str], resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let pkg_json_kind = match resolution_mode { ResolutionMode::Require => deno_package_json::NodeModuleKind::Cjs, ResolutionMode::Import => deno_package_json::NodeModuleKind::Esm, @@ -1434,15 +1447,15 @@ impl< // a corresponding declaration file if let Some(main) = package_json.main(pkg_json_kind) { let main = package_json.path.parent().unwrap().join(main).clean(); - let decl_url_result = self.path_to_declaration_url( - &main, + let decl_path_result = self.path_to_declaration_path( + main, maybe_referrer, resolution_mode, conditions, ); // don't surface errors, fallback to checking the index now - if let Ok(url) = decl_url_result { - return Ok(url); + if let Ok(url_or_path) = decl_path_result { + return Ok(url_or_path); } } None @@ -1455,7 +1468,7 @@ impl< if let Some(main) = maybe_main { let guess = package_json.path.parent().unwrap().join(main).clean(); if self.sys.fs_is_file_no_err(&guess) { - return Ok(url_from_file_path(&guess).unwrap()); + return Ok(UrlOrPath::Path(guess)); } // todo(dsherret): investigate exactly how node and typescript handles this @@ -1485,26 +1498,28 @@ impl< .clean(); if self.sys.fs_is_file_no_err(&guess) { // TODO(bartlomieju): emitLegacyIndexDeprecation() - return Ok(url_from_file_path(&guess).unwrap()); + return Ok(UrlOrPath::Path(guess)); } } } - self.legacy_index_resolve( - package_json.path.parent().unwrap(), - maybe_referrer, - resolution_mode, - resolution_kind, - ) + self + .legacy_index_resolve( + package_json.path.parent().unwrap(), + maybe_referrer, + resolution_mode, + resolution_kind, + ) + .map(UrlOrPath::Path) } fn legacy_index_resolve( &self, directory: &Path, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, resolution_mode: ResolutionMode, resolution_kind: NodeResolutionKind, - ) -> Result { + ) -> Result { let index_file_names = if resolution_kind.is_types() { // todo(dsherret): investigate exactly how typescript does this match resolution_mode { @@ -1520,25 +1535,25 @@ impl< let guess = directory.join(index_file_name).clean(); if self.sys.fs_is_file_no_err(&guess) { // TODO(bartlomieju): emitLegacyIndexDeprecation() - return Ok(url_from_file_path(&guess).unwrap()); + return Ok(guess); } } if resolution_kind.is_types() { Err( TypesNotFoundError(Box::new(TypesNotFoundErrorData { - code_specifier: url_from_file_path(&directory.join("index.js")) - .unwrap(), - maybe_referrer: maybe_referrer.cloned(), + code_specifier: UrlOrPathRef::from_path(&directory.join("index.js")) + .display(), + maybe_referrer: maybe_referrer.map(|r| r.display()), })) .into(), ) } else { Err( ModuleNotFoundError { - specifier: url_from_file_path(&directory.join("index.js")).unwrap(), + specifier: UrlOrPath::Path(directory.join("index.js")), typ: "module", - maybe_referrer: maybe_referrer.cloned(), + maybe_referrer: maybe_referrer.map(|r| r.display()), } .into(), ) @@ -1652,14 +1667,6 @@ fn resolve_bin_entry_value<'a>( } } -fn to_file_path(url: &Url) -> PathBuf { - deno_path_util::url_to_file_path(url).unwrap() -} - -fn to_file_path_string(url: &Url) -> String { - to_file_path(url).display().to_string() -} - fn should_be_treated_as_relative_or_absolute_path(specifier: &str) -> bool { if specifier.is_empty() { return false; @@ -1731,11 +1738,11 @@ fn with_known_extension(path: &Path, ext: &str) -> PathBuf { path.with_file_name(format!("{file_name}.{ext}")) } -fn to_specifier_display_string(url: &Url) -> String { - if let Ok(path) = deno_path_util::url_to_file_path(url) { +fn to_specifier_display_string(url: &UrlOrPathRef) -> String { + if let Ok(path) = url.path() { path.display().to_string() } else { - url.to_string() + url.display().to_string() } } @@ -1743,7 +1750,7 @@ fn throw_invalid_subpath( subpath: String, package_json_path: &Path, internal: bool, - maybe_referrer: Option<&Url>, + maybe_referrer: Option<&UrlOrPathRef>, ) -> InvalidModuleSpecifierError { let ie = if internal { "imports" } else { "exports" }; let reason = format!( @@ -1760,7 +1767,7 @@ fn throw_invalid_subpath( pub fn parse_npm_pkg_name<'a>( specifier: &'a str, - referrer: &Url, + referrer: &UrlOrPathRef, ) -> Result<(&'a str, Cow<'static, str>, bool), InvalidModuleSpecifierError> { let mut separator_index = specifier.find('/'); let mut valid_package_name = true; @@ -2045,6 +2052,7 @@ mod tests { #[test] fn test_parse_package_name() { let dummy_referrer = Url::parse("http://example.com").unwrap(); + let dummy_referrer = UrlOrPathRef::from_url(&dummy_referrer); assert_eq!( parse_npm_pkg_name("fetch-blob", &dummy_referrer).unwrap(),