From 39159f2019cada3bb1448cbc7c58b40fb2631f94 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 30 Jul 2025 15:53:02 -0400 Subject: [PATCH 1/3] feat: ability to resolve npm and jsr specifiers on demand --- src/mod.ts | 37 +++- src/rs_lib/lib.rs | 162 +++++++++++++----- tests/resolve_async/main.test.ts | 49 ++++++ tests/resolve_async/testdata/deno.json | 3 + .../testdata/deno.manual_install.json | 3 + 5 files changed, 209 insertions(+), 45 deletions(-) create mode 100644 tests/resolve_async/main.test.ts create mode 100644 tests/resolve_async/testdata/deno.json create mode 100644 tests/resolve_async/testdata/deno.manual_install.json diff --git a/src/mod.ts b/src/mod.ts index 80c253d..b8972be 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -16,6 +16,8 @@ * if (diagnostics.length > 0) { * throw new Error(diagnostics[0].message); * } + * // alternatively use resolveAsync to resolve npm/jsr specifiers not found + * // in the entrypoints or if not being able to provide entrypoints * const resolvedUrl = loader.resolve( * "./mod.test.ts", * "https://deno.land/mod.ts", // referrer @@ -208,7 +210,7 @@ export class Loader implements Disposable { return messages.map((message) => ({ message })); } - /** Resolves a specifier using the given referrer and resolution mode. */ + /** Synchronously resolves a specifier using the given referrer and resolution mode. */ resolve( specifier: string, referrer: string | undefined, @@ -228,6 +230,39 @@ export class Loader implements Disposable { return value; } + /** Asynchronously resolves a specifier using the given referrer and resolution mode. + * + * This is useful for resolving `jsr:` and `npm:` specifiers on the fly when they can't + * be figured out from entrypoints, but it may cause multiple "npm install"s and different + * npm or jsr resolution than Deno. For that reason it's better to provide the list of + * entrypoints up front so the loader can create the npm and jsr graph, and then after use + * synchronous resolution to resolve jsr and npm specifiers. + */ + resolveAsync( + specifier: string, + referrer: string | undefined, + resolutionMode: ResolutionMode, + ) { + // note: this function breaks typical JS naming conventions because it is + // preferred that people use resolve instead of resolveAsync + if (this.#debug) { + console.error( + `Resolving '${specifier}' from '${referrer ?? ""}' (${ + resolutionModeToString(resolutionMode) + })`, + ); + } + const value = this.#inner.resolve_async( + specifier, + referrer, + resolutionMode, + ); + if (this.#debug) { + console.error(`Resolved to '${value}'`); + } + return value; + } + /** Loads a specifier. */ load( specifier: string, diff --git a/src/rs_lib/lib.rs b/src/rs_lib/lib.rs index 56d9068..3ff74bf 100644 --- a/src/rs_lib/lib.rs +++ b/src/rs_lib/lib.rs @@ -1,5 +1,6 @@ mod http_client; +use std::borrow::Cow; use std::path::Path; use std::path::PathBuf; use std::rc::Rc; @@ -49,6 +50,8 @@ use deno_resolver::loader::ModuleLoader; use deno_resolver::loader::RequestedModuleType; use deno_resolver::npm::DenoInNpmPackageChecker; use deno_semver::SmallStackString; +use deno_semver::jsr::JsrPackageReqReference; +use deno_semver::npm::NpmPackageReqReference; use js_sys::Object; use js_sys::Uint8Array; use node_resolver::NodeConditionOptions; @@ -356,10 +359,32 @@ impl DenoLoader { &mut self, entrypoints: Vec, ) -> Result, anyhow::Error> { - let roots = entrypoints + let urls = entrypoints .into_iter() - .map(|e| self.resolve_entrypoint(e)) + .map(|e| self.resolve_entrypoint(Cow::Owned(e))) .collect::, _>>()?; + self.add_entrypoint_urls(urls.clone()).await?; + let errors = self + .graph + .walk( + urls.iter(), + WalkOptions { + check_js: CheckJsOption::True, + kind: GraphKind::CodeOnly, + follow_dynamic: false, + prefer_fast_check_graph: false, + }, + ) + .errors() + .map(|e| e.to_string_with_range()) + .collect(); + Ok(errors) + } + + async fn add_entrypoint_urls( + &mut self, + entrypoints: Vec, + ) -> Result<(), anyhow::Error> { let npm_package_info_provider = self .npm_installer_factory .lockfile_npm_package_info_provider()?; @@ -396,7 +421,7 @@ impl DenoLoader { self .graph .build( - roots.clone(), + entrypoints, Vec::new(), &loader, deno_graph::BuildOptions { @@ -417,21 +442,7 @@ impl DenoLoader { }, ) .await; - let errors = self - .graph - .walk( - roots.iter(), - WalkOptions { - check_js: CheckJsOption::True, - kind: GraphKind::CodeOnly, - follow_dynamic: false, - prefer_fast_check_graph: false, - }, - ) - .errors() - .map(|e| e.to_string_with_range()) - .collect(); - Ok(errors) + Ok(()) } pub fn resolve( @@ -440,58 +451,113 @@ impl DenoLoader { importer: Option, resolution_mode: u8, ) -> Result { - let resolution_mode = match resolution_mode { - 1 => node_resolver::ResolutionMode::Require, - _ => node_resolver::ResolutionMode::Import, - }; self - .resolve_inner(specifier, importer, resolution_mode) + .resolve_inner( + &specifier, + importer, + parse_resolution_mode(resolution_mode), + ) .map_err(create_js_error) } fn resolve_inner( &self, + specifier: &str, + importer: Option, + resolution_mode: node_resolver::ResolutionMode, + ) -> Result { + let (specifier, referrer) = + self.resolve_specifier_and_referrer(specifier, importer)?; + let resolved = self.resolver.resolve_with_graph( + &self.graph, + &specifier, + &referrer, + deno_graph::Position::zeroed(), + ResolveWithGraphOptions { + mode: resolution_mode, + kind: node_resolver::NodeResolutionKind::Execution, + maintain_npm_specifiers: false, + }, + )?; + Ok(resolved.into()) + } + + pub async fn resolve_async( + &mut self, specifier: String, importer: Option, + resolution_mode: u8, + ) -> Result { + self + .resolve_async_inner( + &specifier, + importer, + parse_resolution_mode(resolution_mode), + ) + .await + .map_err(create_js_error) + } + + async fn resolve_async_inner( + &mut self, + specifier: &str, + importer: Option, resolution_mode: node_resolver::ResolutionMode, ) -> Result { + let (specifier, referrer) = + self.resolve_specifier_and_referrer(&specifier, importer.clone())?; + let resolved = self.resolver.resolve_with_graph( + &self.graph, + &specifier, + &referrer, + deno_graph::Position::zeroed(), + ResolveWithGraphOptions { + mode: resolution_mode, + kind: node_resolver::NodeResolutionKind::Execution, + maintain_npm_specifiers: true, + }, + )?; + if NpmPackageReqReference::from_specifier(&resolved).is_ok() + || JsrPackageReqReference::from_specifier(&resolved).is_ok() + { + self.add_entrypoint_urls(vec![resolved.clone()]).await?; + self.resolve_inner(&specifier, importer, resolution_mode) + } else { + Ok(resolved.into()) + } + } + + fn resolve_specifier_and_referrer<'a>( + &self, + specifier: &'a str, + importer: Option, + ) -> Result<(Cow<'a, str>, Url), anyhow::Error> { let importer = importer.filter(|v| !v.is_empty()); - let (specifier, referrer) = match importer { + Ok(match importer { Some(referrer) if referrer.starts_with("http:") || referrer.starts_with("https:") || referrer.starts_with("file:") => { - (specifier, Url::parse(&referrer)?) + (Cow::Borrowed(specifier), Url::parse(&referrer)?) } Some(referrer) => ( - specifier, + Cow::Borrowed(specifier), deno_path_util::url_from_file_path( &sys_traits::impls::wasm_string_to_path(referrer), )?, ), None => { - let entrypoint = self.resolve_entrypoint(specifier)?.to_string(); + let entrypoint = + Cow::Owned(self.resolve_entrypoint(Cow::Borrowed(specifier))?.into()); ( entrypoint, - deno_path_util::url_from_file_path( + deno_path_util::url_from_directory_path( self.workspace_factory.initial_cwd(), )?, ) } - }; - let resolved = self.resolver.resolve_with_graph( - &self.graph, - &specifier, - &referrer, - deno_graph::Position::zeroed(), - ResolveWithGraphOptions { - mode: resolution_mode, - kind: node_resolver::NodeResolutionKind::Execution, - maintain_npm_specifiers: false, - }, - )?; - Ok(resolved.into()) + }) } pub async fn load( @@ -528,7 +594,7 @@ impl DenoLoader { return Ok(create_external_repsonse(&url)); } else if url.scheme() == "jsr" { bail!( - "Failed loading '{}'. jsr: specifiers must be resolved to an https: specifier before being loaded.", + "Failed loading '{}'. jsr: specifiers must be resolved to an https: specifier before being loaded. Ensure it's found via an entrypoint or resolve with resolveAsync.", url ); } @@ -634,12 +700,13 @@ impl DenoLoader { fn resolve_entrypoint( &self, - specifier: String, + specifier: Cow, ) -> Result { let cwd = self.workspace_factory.initial_cwd(); if specifier.contains('\\') { return Ok(deno_path_util::url_from_file_path(&resolve_absolute_path( - specifier, cwd, + specifier.into_owned(), + cwd, ))?); } let referrer = deno_path_util::url_from_directory_path(cwd)?; @@ -702,6 +769,13 @@ fn create_js_error(err: anyhow::Error) -> JsValue { wasm_bindgen::JsError::new(&err.to_string()).into() } +fn parse_resolution_mode(resolution_mode: u8) -> node_resolver::ResolutionMode { + match resolution_mode { + 1 => node_resolver::ResolutionMode::Require, + _ => node_resolver::ResolutionMode::Import, + } +} + fn media_type_to_u8(media_type: MediaType) -> u8 { match media_type { MediaType::JavaScript => 0, diff --git a/tests/resolve_async/main.test.ts b/tests/resolve_async/main.test.ts new file mode 100644 index 0000000..db77717 --- /dev/null +++ b/tests/resolve_async/main.test.ts @@ -0,0 +1,49 @@ +import { ResolutionMode } from "@deno/loader"; +import { createLoader } from "../helpers.ts"; +import { assert, assertRejects } from "@std/assert"; + +Deno.test("resolves npm specifiers and jsr specifiers on demand with resolveAsync", async () => { + const { loader } = await createLoader({ + configPath: import.meta.dirname + "/testdata/deno.json", + }, { + entrypoints: [], + }); + + { + const jsrUrl = await loader.resolveAsync( + "jsr:@david/code-block-writer", + import.meta.url, + ResolutionMode.Import, + ); + assert(jsrUrl.startsWith("https://")); + } + { + const npmUrl = await loader.resolveAsync( + "npm:code-block-writer", + import.meta.url, + ResolutionMode.Import, + ); + assert(npmUrl.startsWith("file:///")); + } +}); + +Deno.test("errors when using nodeModulesDir: manual and npm package is not installed", async () => { + const { loader } = await createLoader({ + configPath: import.meta.dirname + "/testdata/deno.manual_install.json", + }, { + entrypoints: [], + }); + + { + await assertRejects( + () => + loader.resolveAsync( + "npm:code-block-writer", + import.meta.url, + ResolutionMode.Import, + ), + Error, + "Could not find a matching package for 'npm:code-block-writer' in the node_modules directory.", + ); + } +}); diff --git a/tests/resolve_async/testdata/deno.json b/tests/resolve_async/testdata/deno.json new file mode 100644 index 0000000..358b7d2 --- /dev/null +++ b/tests/resolve_async/testdata/deno.json @@ -0,0 +1,3 @@ +{ + "lock": false +} diff --git a/tests/resolve_async/testdata/deno.manual_install.json b/tests/resolve_async/testdata/deno.manual_install.json new file mode 100644 index 0000000..fde86a1 --- /dev/null +++ b/tests/resolve_async/testdata/deno.manual_install.json @@ -0,0 +1,3 @@ +{ + "nodeModulesDir": "manual" +} From 67df3df7c2ad8e92fe0560b81f02684580e39695 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 30 Jul 2025 15:56:10 -0400 Subject: [PATCH 2/3] update --- src/mod.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mod.ts b/src/mod.ts index b8972be..3007c49 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -242,7 +242,7 @@ export class Loader implements Disposable { specifier: string, referrer: string | undefined, resolutionMode: ResolutionMode, - ) { + ): Promise { // note: this function breaks typical JS naming conventions because it is // preferred that people use resolve instead of resolveAsync if (this.#debug) { From 6f43cb66ed92f352bde40b14dee6ceac7dbc5118 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 30 Jul 2025 16:08:26 -0400 Subject: [PATCH 3/3] rename functions --- src/mod.test.ts | 2 +- src/mod.ts | 14 ++++++-------- src/rs_lib/lib.rs | 16 ++++++++-------- tests/bytes_and_text/main.test.ts | 8 ++++++-- tests/bytes_and_text_npm/main.test.ts | 8 ++++++-- tests/jsx/main.test.ts | 14 +++++++++++--- tests/link_jsr_entrypoint/main.test.ts | 2 +- tests/resolve_async/main.test.ts | 6 +++--- 8 files changed, 42 insertions(+), 28 deletions(-) diff --git a/src/mod.test.ts b/src/mod.test.ts index b1b1963..04094a9 100644 --- a/src/mod.test.ts +++ b/src/mod.test.ts @@ -17,7 +17,7 @@ Deno.test("should resolve, load and get graph", async () => { assertEquals(diagnostics.length, 0); const graph = loader.getGraphUnstable(); assertEquals((graph as any).roots[0], modFileUrl); - const resolvedUrl = loader.resolve( + const resolvedUrl = loader.resolveSync( "./mod.test.ts", modFileUrl, ResolutionMode.Import, diff --git a/src/mod.ts b/src/mod.ts index 3007c49..5a9e527 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -16,9 +16,9 @@ * if (diagnostics.length > 0) { * throw new Error(diagnostics[0].message); * } - * // alternatively use resolveAsync to resolve npm/jsr specifiers not found + * // alternatively use resolve to resolve npm/jsr specifiers not found * // in the entrypoints or if not being able to provide entrypoints - * const resolvedUrl = loader.resolve( + * const resolvedUrl = loader.resolveSync( * "./mod.test.ts", * "https://deno.land/mod.ts", // referrer * ResolutionMode.Import, @@ -211,7 +211,7 @@ export class Loader implements Disposable { } /** Synchronously resolves a specifier using the given referrer and resolution mode. */ - resolve( + resolveSync( specifier: string, referrer: string | undefined, resolutionMode: ResolutionMode, @@ -223,7 +223,7 @@ export class Loader implements Disposable { })`, ); } - const value = this.#inner.resolve(specifier, referrer, resolutionMode); + const value = this.#inner.resolve_sync(specifier, referrer, resolutionMode); if (this.#debug) { console.error(`Resolved to '${value}'`); } @@ -238,13 +238,11 @@ export class Loader implements Disposable { * entrypoints up front so the loader can create the npm and jsr graph, and then after use * synchronous resolution to resolve jsr and npm specifiers. */ - resolveAsync( + resolve( specifier: string, referrer: string | undefined, resolutionMode: ResolutionMode, ): Promise { - // note: this function breaks typical JS naming conventions because it is - // preferred that people use resolve instead of resolveAsync if (this.#debug) { console.error( `Resolving '${specifier}' from '${referrer ?? ""}' (${ @@ -252,7 +250,7 @@ export class Loader implements Disposable { })`, ); } - const value = this.#inner.resolve_async( + const value = this.#inner.resolve( specifier, referrer, resolutionMode, diff --git a/src/rs_lib/lib.rs b/src/rs_lib/lib.rs index 3ff74bf..0326c3d 100644 --- a/src/rs_lib/lib.rs +++ b/src/rs_lib/lib.rs @@ -445,14 +445,14 @@ impl DenoLoader { Ok(()) } - pub fn resolve( + pub fn resolve_sync( &self, specifier: String, importer: Option, resolution_mode: u8, ) -> Result { self - .resolve_inner( + .resolve_sync_inner( &specifier, importer, parse_resolution_mode(resolution_mode), @@ -460,7 +460,7 @@ impl DenoLoader { .map_err(create_js_error) } - fn resolve_inner( + fn resolve_sync_inner( &self, specifier: &str, importer: Option, @@ -482,14 +482,14 @@ impl DenoLoader { Ok(resolved.into()) } - pub async fn resolve_async( + pub async fn resolve( &mut self, specifier: String, importer: Option, resolution_mode: u8, ) -> Result { self - .resolve_async_inner( + .resolve_inner( &specifier, importer, parse_resolution_mode(resolution_mode), @@ -498,7 +498,7 @@ impl DenoLoader { .map_err(create_js_error) } - async fn resolve_async_inner( + async fn resolve_inner( &mut self, specifier: &str, importer: Option, @@ -521,7 +521,7 @@ impl DenoLoader { || JsrPackageReqReference::from_specifier(&resolved).is_ok() { self.add_entrypoint_urls(vec![resolved.clone()]).await?; - self.resolve_inner(&specifier, importer, resolution_mode) + self.resolve_sync_inner(&specifier, importer, resolution_mode) } else { Ok(resolved.into()) } @@ -594,7 +594,7 @@ impl DenoLoader { return Ok(create_external_repsonse(&url)); } else if url.scheme() == "jsr" { bail!( - "Failed loading '{}'. jsr: specifiers must be resolved to an https: specifier before being loaded. Ensure it's found via an entrypoint or resolve with resolveAsync.", + "Failed loading '{}'. jsr: specifiers must be resolved to an https: specifier before being loaded.", url ); } diff --git a/tests/bytes_and_text/main.test.ts b/tests/bytes_and_text/main.test.ts index d9edb46..52b933b 100644 --- a/tests/bytes_and_text/main.test.ts +++ b/tests/bytes_and_text/main.test.ts @@ -19,8 +19,12 @@ Deno.test("loads jsx transpiled", async () => { }; const { loader } = await createWorkspace(); - const mainTsUrl = loader.resolve(mainTs, undefined, ResolutionMode.Import); - const dataFileUrl = loader.resolve( + const mainTsUrl = loader.resolveSync( + mainTs, + undefined, + ResolutionMode.Import, + ); + const dataFileUrl = loader.resolveSync( "./data_utf8_bom.txt", mainTsUrl, ResolutionMode.Import, diff --git a/tests/bytes_and_text_npm/main.test.ts b/tests/bytes_and_text_npm/main.test.ts index a707a2e..07b3f2f 100644 --- a/tests/bytes_and_text_npm/main.test.ts +++ b/tests/bytes_and_text_npm/main.test.ts @@ -19,8 +19,12 @@ Deno.test("loads jsx transpiled", async () => { }; const { loader } = await createWorkspace(); - const mainTsUrl = loader.resolve(mainTs, undefined, ResolutionMode.Import); - const dataFileUrl = loader.resolve( + const mainTsUrl = loader.resolveSync( + mainTs, + undefined, + ResolutionMode.Import, + ); + const dataFileUrl = loader.resolveSync( "package/data.txt", mainTsUrl, ResolutionMode.Import, diff --git a/tests/jsx/main.test.ts b/tests/jsx/main.test.ts index 7aa48e7..dc1abfc 100644 --- a/tests/jsx/main.test.ts +++ b/tests/jsx/main.test.ts @@ -24,8 +24,16 @@ Deno.test("loads jsx transpiled", async () => { "//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIm1haW4uanN4Il0sInNvdXJjZXNDb250ZW50IjpbImNvbnNvbGUubG9nKDxkaXYgLz4pO1xuIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiI7Ozs7QUFBQSxRQUFRLEdBQUcifQ=="; const mainTsxSourceMappingURL = "//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbIm1haW4udHN4Il0sInNvdXJjZXNDb250ZW50IjpbImNvbnN0IHZhbHVlOiBzdHJpbmcgPSBcIlwiO1xuY29uc29sZS5sb2coPGRpdiAvPiwgdmFsdWUpO1xuIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiJBQUFBLE1BQU0sUUFBZ0I7QUFDdEIsUUFBUSxHQUFHLEVBQUUsT0FBUSJ9"; - const mainJsxUrl = loader.resolve(mainJsx, undefined, ResolutionMode.Import); - const mainTsxUrl = loader.resolve(mainTsx, undefined, ResolutionMode.Import); + const mainJsxUrl = loader.resolveSync( + mainJsx, + undefined, + ResolutionMode.Import, + ); + const mainTsxUrl = loader.resolveSync( + mainTsx, + undefined, + ResolutionMode.Import, + ); assertResponseText( await loader.load(mainJsxUrl, RequestedModuleType.Default), @@ -38,7 +46,7 @@ ${mainJsxSourceMappingURL}`, ); // resolves jsx-dev - const jsx = loader.resolve( + const jsx = loader.resolveSync( "react/jsx-dev-runtime", mainTsx, ResolutionMode.Import, diff --git a/tests/link_jsr_entrypoint/main.test.ts b/tests/link_jsr_entrypoint/main.test.ts index 570be1c..d516ed8 100644 --- a/tests/link_jsr_entrypoint/main.test.ts +++ b/tests/link_jsr_entrypoint/main.test.ts @@ -14,7 +14,7 @@ Deno.test("loads linked entrypoint", async () => { }); const response = await loader.load( - loader.resolve("@denotest/add", undefined, ResolutionMode.Import), + loader.resolveSync("@denotest/add", undefined, ResolutionMode.Import), RequestedModuleType.Default, ); assertResponseText( diff --git a/tests/resolve_async/main.test.ts b/tests/resolve_async/main.test.ts index db77717..1595e8b 100644 --- a/tests/resolve_async/main.test.ts +++ b/tests/resolve_async/main.test.ts @@ -10,7 +10,7 @@ Deno.test("resolves npm specifiers and jsr specifiers on demand with resolveAsyn }); { - const jsrUrl = await loader.resolveAsync( + const jsrUrl = await loader.resolve( "jsr:@david/code-block-writer", import.meta.url, ResolutionMode.Import, @@ -18,7 +18,7 @@ Deno.test("resolves npm specifiers and jsr specifiers on demand with resolveAsyn assert(jsrUrl.startsWith("https://")); } { - const npmUrl = await loader.resolveAsync( + const npmUrl = await loader.resolve( "npm:code-block-writer", import.meta.url, ResolutionMode.Import, @@ -37,7 +37,7 @@ Deno.test("errors when using nodeModulesDir: manual and npm package is not insta { await assertRejects( () => - loader.resolveAsync( + loader.resolve( "npm:code-block-writer", import.meta.url, ResolutionMode.Import,