From 68fb0d0f6c83b5e17de1605c7e49d54617a938c6 Mon Sep 17 00:00:00 2001 From: leaysgur <6259812+leaysgur@users.noreply.github.com> Date: Fri, 13 Mar 2026 05:24:03 +0000 Subject: [PATCH] fix(oxfmt): Skip vite.config.ts which fails to import (#20326) If oxfmt.config.ts fails to load, it is the user's responsibility, so reporting it as an error is correct. However, this does not apply to vite.config.ts, so we ignore it. Also refactored to report error message from JS. --- apps/oxfmt/src-js/cli/js_config.ts | 6 ++-- apps/oxfmt/src/core/config.rs | 30 +++++++++++-------- apps/oxfmt/src/core/js_config.rs | 3 +- .../__snapshots__/js_config.test.ts.snap | 2 ++ .../__snapshots__/vite_config.test.ts.snap | 17 +++++++++++ .../fixtures/error_finds_parent/.oxfmtrc.json | 3 ++ .../fixtures/error_finds_parent/child/test.ts | 1 + .../error_finds_parent/child/vite.config.ts | 7 +++++ .../test/cli/vite_config/vite_config.test.ts | 9 ++++++ 9 files changed, 61 insertions(+), 17 deletions(-) create mode 100644 apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/.oxfmtrc.json create mode 100644 apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/test.ts create mode 100644 apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/vite.config.ts diff --git a/apps/oxfmt/src-js/cli/js_config.ts b/apps/oxfmt/src-js/cli/js_config.ts index e7822b255223c..4f329c3610420 100644 --- a/apps/oxfmt/src-js/cli/js_config.ts +++ b/apps/oxfmt/src-js/cli/js_config.ts @@ -24,7 +24,7 @@ export async function loadJsConfig(path: string): Promise { fileUrl.searchParams.set("cache", Date.now().toString()); const { default: config } = await import(fileUrl.href); - if (config === undefined) throw new Error(`Configuration file has no default export: ${path}`); + if (config === undefined) throw new Error("Configuration file has no default export."); // Vite config: extract `.fmt` field if (pathBasename(path) === VITE_CONFIG_NAME) { @@ -39,14 +39,14 @@ export async function loadJsConfig(path: string): Promise { if (!isObject(fmtConfig)) { throw new Error( - `The \`${VITE_OXFMT_CONFIG_FIELD}\` field in the default export must be an object: ${path}`, + `The \`${VITE_OXFMT_CONFIG_FIELD}\` field in the default export must be an object.`, ); } return fmtConfig; } if (!isObject(config)) { - throw new Error(`Configuration file must have a default export that is an object: ${path}`); + throw new Error("Configuration file must have a default export that is an object."); } return config; diff --git a/apps/oxfmt/src/core/config.rs b/apps/oxfmt/src/core/config.rs index 59cd600de3cb7..7a649d3ddbff1 100644 --- a/apps/oxfmt/src/core/config.rs +++ b/apps/oxfmt/src/core/config.rs @@ -6,6 +6,8 @@ use editorconfig_parser::{ }; use fast_glob::glob_match; use serde_json::Value; +#[cfg(feature = "napi")] +use tracing::debug; use tracing::instrument; use oxc_formatter::FormatOptions; @@ -301,20 +303,24 @@ impl ConfigResolver { // For `vite.config.ts` #[cfg(feature = "napi")] if is_vite_plus_config(&path) { - if let Some(raw_config) = load_js_config( + match load_js_config( js_config_loader .expect("JS config loader must be set when `napi` feature is enabled"), &path, - )? { - let editorconfig = load_editorconfig(cwd, editorconfig_path)?; - return Ok(Self::new( - raw_config, - path.parent().map(Path::to_path_buf), - editorconfig, - )); + ) { + // Load successful and `.fmt` field found -> Use it as config + Ok(Some(raw_config)) => { + let editorconfig = load_editorconfig(cwd, editorconfig_path)?; + let config_dir = path.parent().map(Path::to_path_buf); + return Ok(Self::new(raw_config, config_dir, editorconfig)); + } + // Load successful but no `.fmt` field + // -> Skip and continue searching, otherwise `load_config_at()` would treat as an error + Ok(None) => debug!("No `.fmt` field in {}, skipping", path.display()), + // Load failed (e.g., syntax error, missing dependencies) + // -> Skip and continue searching, as the config file is likely not intended for Oxfmt + Err(err) => debug!("Failed to load {}: {err}, skipping", path.display()), } - // `load_js_config()` returns `None` if `.fmt` is missing. - // Skip it and continue, otherwise `load_config_at()` would treat as an error. continue; } @@ -492,9 +498,9 @@ fn load_js_config( js_config_loader: &JsConfigLoaderCb, path: &Path, ) -> Result, String> { - let value = js_config_loader(path.to_string_lossy().into_owned()).map_err(|_| { + let value = js_config_loader(path.to_string_lossy().into_owned()).map_err(|err| { format!( - "{}\nEnsure the file has a valid default export of a JSON-serializable configuration object.", + "{}\n{err}\nEnsure the file has a valid default export of a JSON-serializable configuration object.", path.display() ) })?; diff --git a/apps/oxfmt/src/core/js_config.rs b/apps/oxfmt/src/core/js_config.rs index 8bc59791f3ae4..8598ff4b37b2d 100644 --- a/apps/oxfmt/src/core/js_config.rs +++ b/apps/oxfmt/src/core/js_config.rs @@ -41,7 +41,6 @@ pub fn create_js_config_loader(cb: JsLoadJsConfigCb) -> JsConfigLoaderCb { tokio::runtime::Handle::current() .block_on(async move { cb.call_async(path).await?.into_future().await }) }); - - res.map_err(|_| "failed to load".to_string()) + res.map_err(|e| e.reason.clone()) }) } diff --git a/apps/oxfmt/test/cli/js_config/__snapshots__/js_config.test.ts.snap b/apps/oxfmt/test/cli/js_config/__snapshots__/js_config.test.ts.snap index f5af81f8f2df1..14b238ecaf5db 100644 --- a/apps/oxfmt/test/cli/js_config/__snapshots__/js_config.test.ts.snap +++ b/apps/oxfmt/test/cli/js_config/__snapshots__/js_config.test.ts.snap @@ -27,6 +27,7 @@ exit code: 1 --- STDERR --------- Failed to load configuration file. /oxfmt.config.ts +Error: Configuration file must have a default export that is an object. Ensure the file has a valid default export of a JSON-serializable configuration object. --------------------" `; @@ -41,6 +42,7 @@ exit code: 1 --- STDERR --------- Failed to load configuration file. /oxfmt.config.ts +Error: Configuration file has no default export. Ensure the file has a valid default export of a JSON-serializable configuration object. --------------------" `; diff --git a/apps/oxfmt/test/cli/vite_config/__snapshots__/vite_config.test.ts.snap b/apps/oxfmt/test/cli/vite_config/__snapshots__/vite_config.test.ts.snap index 922f266a834c9..fddbc77b8ae8a 100644 --- a/apps/oxfmt/test/cli/vite_config/__snapshots__/vite_config.test.ts.snap +++ b/apps/oxfmt/test/cli/vite_config/__snapshots__/vite_config.test.ts.snap @@ -77,6 +77,23 @@ Finished in ms on 1 files using 1 threads. --------------------" `; +exports[`vite_config > skip: parent config is found when vite.config.ts fails to load 1`] = ` +"-------------------- +arguments: --check test.ts +working directory: vite_config/fixtures/error_finds_parent/child +exit code: 1 +--- STDOUT --------- +Checking formatting... + +test.ts (ms) + +Format issues found in above 1 files. Run without \`--check\` to fix. +Finished in ms on 1 files using 1 threads. +--- STDERR --------- + +--------------------" +`; + exports[`vite_config > skip: parent config is found when vite.config.ts without fmt is skipped 1`] = ` "-------------------- arguments: --check test.ts diff --git a/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/.oxfmtrc.json b/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/.oxfmtrc.json new file mode 100644 index 0000000000000..cce9d3c080177 --- /dev/null +++ b/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/.oxfmtrc.json @@ -0,0 +1,3 @@ +{ + "semi": false +} diff --git a/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/test.ts b/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/test.ts new file mode 100644 index 0000000000000..54b82a09ad543 --- /dev/null +++ b/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/test.ts @@ -0,0 +1 @@ +const a = 1; diff --git a/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/vite.config.ts b/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/vite.config.ts new file mode 100644 index 0000000000000..25e98ad656232 --- /dev/null +++ b/apps/oxfmt/test/cli/vite_config/fixtures/error_finds_parent/child/vite.config.ts @@ -0,0 +1,7 @@ +import "non-existent-module"; + +export default { + fmt: { + semi: true, + }, +}; diff --git a/apps/oxfmt/test/cli/vite_config/vite_config.test.ts b/apps/oxfmt/test/cli/vite_config/vite_config.test.ts index d8edf11b7b3bd..ad1a14963ca25 100644 --- a/apps/oxfmt/test/cli/vite_config/vite_config.test.ts +++ b/apps/oxfmt/test/cli/vite_config/vite_config.test.ts @@ -25,6 +25,15 @@ describe("vite_config", () => { expect(snapshot).toMatchSnapshot(); }); + it("skip: parent config is found when vite.config.ts fails to load", async () => { + // child/ has vite.config.ts that imports a non-existent module → load error → skipped + // parent has .oxfmtrc.json with semi: false + // So `const a = 1;` (with semicolon) should be flagged as mismatch + const cwd = join(fixturesDir, "error_finds_parent", "child"); + const snapshot = await runAndSnapshot(cwd, [["--check", "test.ts"]]); + expect(snapshot).toMatchSnapshot(); + }); + it("skip: parent config is found when vite.config.ts without fmt is skipped", async () => { // child/ has vite.config.ts without .fmt → skipped // parent has .oxfmtrc.json with semi: false