From eef171fefdd544a8b33ed8ab21d56b0a55733912 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 30 Jul 2025 14:15:55 -0400 Subject: [PATCH 1/2] feat: return graph errors as diagnostics --- src/mod.test.ts | 6 ++++-- src/mod.ts | 24 ++++++++++++++++++------ src/rs_lib/lib.rs | 26 +++++++++++++++++++++----- tests/helpers.ts | 3 ++- tests/jsx/main.test.ts | 8 +++++--- 5 files changed, 50 insertions(+), 17 deletions(-) diff --git a/src/mod.test.ts b/src/mod.test.ts index 3a555de..b1b1963 100644 --- a/src/mod.test.ts +++ b/src/mod.test.ts @@ -11,9 +11,10 @@ Deno.test("should resolve, load and get graph", async () => { nodeConditions: undefined, // ensure doesn't error }); const modFileUrl = import.meta.resolve("./mod.ts"); - const loader = await workspace.createLoader({ + const { loader, diagnostics } = await workspace.createLoader({ entrypoints: [modFileUrl], }); + assertEquals(diagnostics.length, 0); const graph = loader.getGraphUnstable(); assertEquals((graph as any).roots[0], modFileUrl); const resolvedUrl = loader.resolve( @@ -51,9 +52,10 @@ Deno.test("should resolve, load and get graph", async () => { Deno.test("resolving a jsr specifier should fail with explanatory message", async () => { const workspace = new Workspace({}); const modFileUrl = import.meta.resolve("./mod.ts"); - const loader = await workspace.createLoader({ + const { loader, diagnostics } = await workspace.createLoader({ entrypoints: [modFileUrl], }); + assertEquals(diagnostics.length, 0); assertRejects( async () => { await loader.load( diff --git a/src/mod.ts b/src/mod.ts index c2b18f4..80c253d 100644 --- a/src/mod.ts +++ b/src/mod.ts @@ -10,9 +10,12 @@ * const workspace = new Workspace({ * // optional options * }); - * const loader = await workspace.createLoader({ + * const { loader, diagnostics } = await workspace.createLoader({ * entrypoints: ["./mod.ts"] * }); + * if (diagnostics.length > 0) { + * throw new Error(diagnostics[0].message); + * } * const resolvedUrl = loader.resolve( * "./mod.test.ts", * "https://deno.land/mod.ts", // referrer @@ -146,7 +149,9 @@ export class Workspace implements Disposable { } /** Creates a loader that uses this this workspace. */ - async createLoader(options: LoaderOptions): Promise { + async createLoader( + options: LoaderOptions, + ): Promise<{ loader: Loader; diagnostics: EntrypointDiagnostic[] }> { if (this.#debug) { console.error( `Creating loader for entrypoints:\n ${ @@ -156,8 +161,8 @@ export class Workspace implements Disposable { } const wasmLoader = await this.#inner.create_loader(); const loader = new Loader(wasmLoader, this.#debug); - await loader.addEntrypoints(options.entrypoints); - return loader; + const diagnostics = await loader.addEntrypoints(options.entrypoints); + return { loader, diagnostics }; } } @@ -168,6 +173,10 @@ export enum RequestedModuleType { Bytes = 3, } +export interface EntrypointDiagnostic { + message: string; +} + /** A loader for resolving and loading urls. */ export class Loader implements Disposable { #inner: WasmLoader; @@ -192,8 +201,11 @@ export class Loader implements Disposable { * stored in the internal module graph on the fly, which will allow * it to be synchronously resolved. */ - addEntrypoints(entrypoints: string[]): Promise { - return this.#inner.add_entrypoints(entrypoints); + async addEntrypoints( + entrypoints: string[], + ): Promise { + const messages = await this.#inner.add_entrypoints(entrypoints); + return messages.map((message) => ({ message })); } /** Resolves a specifier using the given referrer and resolution mode. */ diff --git a/src/rs_lib/lib.rs b/src/rs_lib/lib.rs index 0258e9f..56d9068 100644 --- a/src/rs_lib/lib.rs +++ b/src/rs_lib/lib.rs @@ -10,9 +10,12 @@ use deno_ast::ModuleKind; use deno_cache_dir::file_fetcher::CacheSetting; use deno_cache_dir::file_fetcher::NullBlobStore; use deno_error::JsErrorBox; +use deno_graph::CheckJsOption; +use deno_graph::GraphKind; use deno_graph::MediaType; use deno_graph::ModuleGraph; use deno_graph::Position; +use deno_graph::WalkOptions; use deno_graph::analysis::ModuleAnalyzer; use deno_graph::ast::CapturingEsParser; use deno_graph::ast::DefaultEsParser; @@ -336,7 +339,7 @@ impl DenoLoader { pub async fn add_entrypoints( &mut self, entrypoints: Vec, - ) -> Result<(), JsValue> { + ) -> Result, JsValue> { // only allow one async task to modify the graph at a time let task_queue = self.task_queue.clone(); task_queue @@ -352,7 +355,7 @@ impl DenoLoader { async fn add_entrypoints_internal( &mut self, entrypoints: Vec, - ) -> Result<(), anyhow::Error> { + ) -> Result, anyhow::Error> { let roots = entrypoints .into_iter() .map(|e| self.resolve_entrypoint(e)) @@ -393,7 +396,7 @@ impl DenoLoader { self .graph .build( - roots, + roots.clone(), Vec::new(), &loader, deno_graph::BuildOptions { @@ -414,8 +417,21 @@ impl DenoLoader { }, ) .await; - self.graph.valid()?; - Ok(()) + 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) } pub fn resolve( diff --git a/tests/helpers.ts b/tests/helpers.ts index a58a575..01f601a 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -14,7 +14,8 @@ export async function createLoader( loaderOptions: LoaderOptions, ) { const workspace = new Workspace(workspaceOptions); - const loader = await workspace.createLoader(loaderOptions); + const { loader, diagnostics } = await workspace.createLoader(loaderOptions); + assertEquals(diagnostics, []); return { loader, workspace, diff --git a/tests/jsx/main.test.ts b/tests/jsx/main.test.ts index fc803c6..7aa48e7 100644 --- a/tests/jsx/main.test.ts +++ b/tests/jsx/main.test.ts @@ -5,7 +5,7 @@ import { ResolutionMode, type WorkspaceOptions, } from "../helpers.ts"; -import { assert } from "@std/assert"; +import { assert, assertEquals } from "@std/assert"; Deno.test("loads jsx transpiled", async () => { const mainJsx = import.meta.dirname + "/testdata/main.jsx"; @@ -47,9 +47,10 @@ ${mainJsxSourceMappingURL}`, { const { workspace } = await createWorkspace({ preserveJsx: true }); - const newLoader = await workspace.createLoader({ + const { loader: newLoader, diagnostics } = await workspace.createLoader({ entrypoints: [mainJsx, mainTsxUrl], }); + assertEquals(diagnostics, []); assertResponseText( await newLoader.load(mainJsxUrl, RequestedModuleType.Default), "console.log(
);\n", @@ -61,9 +62,10 @@ ${mainJsxSourceMappingURL}`, } { const { workspace } = await createWorkspace({ noTranspile: true }); - const newLoader = await workspace.createLoader({ + const { loader: newLoader, diagnostics } = await workspace.createLoader({ entrypoints: [mainJsx, mainTsx], }); + assertEquals(diagnostics, []); assertResponseText( await newLoader.load(mainJsxUrl, RequestedModuleType.Default), `console.log(
);\n`, From 994e989818a256fc4cbfcd2a583a79c41fc500b0 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Wed, 30 Jul 2025 14:19:51 -0400 Subject: [PATCH 2/2] add test --- tests/helpers.ts | 17 ++++++++++++++++- tests/invalid_graph/main.test.ts | 19 +++++++++++++++++++ tests/invalid_graph/testdata/deno.json | 2 ++ tests/invalid_graph/testdata/main.ts | 1 + 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 tests/invalid_graph/main.test.ts create mode 100644 tests/invalid_graph/testdata/deno.json create mode 100644 tests/invalid_graph/testdata/main.ts diff --git a/tests/helpers.ts b/tests/helpers.ts index 01f601a..ca7381f 100644 --- a/tests/helpers.ts +++ b/tests/helpers.ts @@ -12,13 +12,28 @@ export * from "@deno/loader"; export async function createLoader( workspaceOptions: WorkspaceOptions, loaderOptions: LoaderOptions, +) { + const { loader, workspace, diagnostics } = await createLoaderWithDiagnostics( + workspaceOptions, + loaderOptions, + ); + assertEquals(diagnostics, []); + return { + loader, + workspace, + }; +} + +export async function createLoaderWithDiagnostics( + workspaceOptions: WorkspaceOptions, + loaderOptions: LoaderOptions, ) { const workspace = new Workspace(workspaceOptions); const { loader, diagnostics } = await workspace.createLoader(loaderOptions); - assertEquals(diagnostics, []); return { loader, workspace, + diagnostics, }; } diff --git a/tests/invalid_graph/main.test.ts b/tests/invalid_graph/main.test.ts new file mode 100644 index 0000000..abaa47c --- /dev/null +++ b/tests/invalid_graph/main.test.ts @@ -0,0 +1,19 @@ +import { assertEquals } from "@std/assert"; +import { createLoaderWithDiagnostics } from "../helpers.ts"; + +Deno.test("loads linked entrypoint", async () => { + const mainFile = import.meta.dirname + "/testdata/main.ts"; + const { diagnostics } = await createLoaderWithDiagnostics({ + configPath: import.meta.dirname + "/testdata/deno.json", + }, { + entrypoints: [mainFile], + }); + + assertEquals(diagnostics.length, 1); + const expectedMessage = + 'Relative import path "unknown" not prefixed with / or ./ or ../'; + assertEquals( + diagnostics[0].message.substring(0, expectedMessage.length), + expectedMessage, + ); +}); diff --git a/tests/invalid_graph/testdata/deno.json b/tests/invalid_graph/testdata/deno.json new file mode 100644 index 0000000..2c63c08 --- /dev/null +++ b/tests/invalid_graph/testdata/deno.json @@ -0,0 +1,2 @@ +{ +} diff --git a/tests/invalid_graph/testdata/main.ts b/tests/invalid_graph/testdata/main.ts new file mode 100644 index 0000000..11e0d9e --- /dev/null +++ b/tests/invalid_graph/testdata/main.ts @@ -0,0 +1 @@ +import "unknown";