Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions apps/oxfmt/src-js/bindings.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export declare const enum Severity {
* # Panics
* Panics if the current working directory cannot be determined.
*/
export declare function format(filename: string, sourceText: string, options: any | undefined | null, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<FormatResult>
export declare function format(filename: string, sourceText: string, options: any | undefined | null, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<FormatResult>

export interface FormatResult {
/** The formatted code. */
Expand All @@ -49,7 +49,7 @@ export interface FormatResult {
* This API is specialized for JS/TS snippets embedded in non-JS files.
* Unlike `format()`, it is called only for js-in-xxx `textToDoc()` flow.
*/
export declare function jsTextToDoc(sourceExt: string, sourceText: string, oxfmtPluginOptionsJson: string, parentContext: string, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<string | null>
export declare function jsTextToDoc(sourceExt: string, sourceText: string, oxfmtPluginOptionsJson: string, parentContext: string, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<string | null>

/**
* NAPI based JS CLI entry point.
Expand All @@ -66,4 +66,4 @@ export declare function jsTextToDoc(sourceExt: string, sourceText: string, oxfmt
* - `mode`: If main logic will run in JS side, use this to indicate which mode
* - `exitCode`: If main logic already ran in Rust side, return the exit code
*/
export declare function runCli(args: Array<string>, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindcssClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<[string, number | undefined | null]>
export declare function runCli(args: Array<string>, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindcssClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<[string, number | undefined | null]>
9 changes: 0 additions & 9 deletions apps/oxfmt/src-js/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,4 @@ void (async () => {
// `process.exit()` kills the process immediately and `stdout` may not be flushed before process dies.
// https://nodejs.org/api/process.html#processexitcode
process.exitCode = exitCode!;

// Node.js < 25.4.0 has a race condition with ThreadsafeFunction cleanup that causes
// crashes on large codebases. Add a small delay to allow pending NAPI operations
// to complete before exit. Fixed in Node.js 25.4.0+.
// See: https://github.com/nodejs/node/issues/55706
const [major, minor] = process.versions.node.split(".").map(Number);
if (major < 25 || (major === 25 && minor < 4)) {
setTimeout(() => process.exit(), 50);
}
})();
25 changes: 19 additions & 6 deletions apps/oxfmt/src-js/cli/worker-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import Tinypool from "tinypool";
import { resolvePlugins } from "../libs/apis";
import type {
FormatEmbeddedCodeParam,
FormatEmbeddedCodeResult,
FormatFileParam,
SortTailwindClassesArgs,
} from "../libs/apis";
Expand Down Expand Up @@ -34,10 +35,13 @@ export async function disposeExternalFormatter(): Promise<void> {
export async function formatEmbeddedCode(
options: FormatEmbeddedCodeParam["options"],
code: string,
): Promise<string> {
): Promise<FormatEmbeddedCodeResult> {
return pool!
.run({ options, code } satisfies FormatEmbeddedCodeParam, { name: "formatEmbeddedCode" })
.catch(rethrowAsError);
.run({ options, code } satisfies FormatEmbeddedCodeParam, {
name: "formatEmbeddedCode",
})
.then((formatted) => ({ ok: true, code: formatted }))
.catch((err) => ({ ok: false, error: errorToMessage(err) }));
}

export async function formatFile(
Expand Down Expand Up @@ -68,10 +72,19 @@ export async function sortTailwindClasses(
function rethrowAsError(err: unknown): never {
if (err instanceof Error) throw err;
if (err !== null && typeof err === "object") {
const obj = err as { name: string; message: string };
const newErr = new Error(obj.message);
newErr.name = obj.name;
const obj = err as { name?: unknown; message?: unknown };
const newErr = new Error(errorToMessage(err));
if (typeof obj.name === "string") newErr.name = obj.name;
throw newErr;
}
throw new Error(String(err));
}

function errorToMessage(err: unknown): string {
if (err instanceof Error) return err.message;
if (err !== null && typeof err === "object") {
const message = (err as { message?: unknown }).message;
if (typeof message === "string") return message;
}
return String(err);
}
11 changes: 8 additions & 3 deletions apps/oxfmt/src-js/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { format as napiFormat, jsTextToDoc as napiJsTextToDoc } from "./bindings";
import { resolvePlugins, formatEmbeddedCode, formatFile, sortTailwindClasses } from "./libs/apis";
import {
resolvePlugins,
formatEmbeddedCodeSafe,
formatFile,
sortTailwindClasses,
} from "./libs/apis";
import type { Options } from "prettier";

// napi-JS `oxfmt` API entry point
Expand All @@ -17,7 +22,7 @@ export async function format(fileName: string, sourceText: string, options?: For
sourceText,
options ?? {},
resolvePlugins,
(options, code) => formatEmbeddedCode({ options, code }),
(options, code) => formatEmbeddedCodeSafe({ options, code }),
(options, code) => formatFile({ options, code }),
(options, classes) => sortTailwindClasses({ options, classes }),
);
Expand All @@ -38,7 +43,7 @@ export async function jsTextToDoc(
oxfmtPluginOptionsJson,
parentContext,
resolvePlugins,
(options, code) => formatEmbeddedCode({ options, code }),
(options, code) => formatEmbeddedCodeSafe({ options, code }),
(_options, _code) => Promise.reject(/* Unreachable */),
(options, classes) => sortTailwindClasses({ options, classes }),
);
Expand Down
26 changes: 26 additions & 0 deletions apps/oxfmt/src-js/libs/apis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ export type FormatEmbeddedCodeParam = {
options: Options;
};

export type FormatEmbeddedCodeResult = { ok: true; code: string } | { ok: false; error: string };

/**
* Format xxx-in-js code snippets
*
Expand All @@ -72,6 +74,21 @@ export async function formatEmbeddedCode({
return prettier.format(code, options);
}

/**
* `formatEmbeddedCode()` wrapper that never rejects.
* Rust side receives a resolved object and handles fallback behavior there.
*/
export async function formatEmbeddedCodeSafe(
args: FormatEmbeddedCodeParam,
): Promise<FormatEmbeddedCodeResult> {
try {
const code = await formatEmbeddedCode(args);
return { ok: true, code };
} catch (err) {
return { ok: false, error: errorToMessage(err) };
}
}

// ---

export type FormatFileParam = {
Expand Down Expand Up @@ -191,3 +208,12 @@ async function setupOxfmtPlugin(options: Options): Promise<void> {
options.plugins ??= [];
options.plugins.push(oxcPlugin);
}

function errorToMessage(err: unknown): string {
if (err instanceof Error) return err.message;
if (err !== null && typeof err === "object") {
const message = (err as { message?: unknown }).message;
if (typeof message === "string") return message;
}
return String(err);
}
41 changes: 37 additions & 4 deletions apps/oxfmt/src/core/external_formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ pub type JsInitExternalFormatterCb = ThreadsafeFunction<
>;

/// Type alias for the callback function signature.
/// Takes (options, code) as arguments and returns formatted code.
/// Takes (options, code) as arguments and returns a wrapped result.
/// The `options` object includes `parser` field set by Rust side.
pub type JsFormatEmbeddedCb = ThreadsafeFunction<
// Input arguments
FnArgs<(Value, String)>, // (options, code)
// Return type (what JS function returns)
Promise<String>,
Promise<Value>,
// Arguments (repeated)
FnArgs<(Value, String)>,
// Error status
Expand Down Expand Up @@ -141,7 +141,7 @@ impl std::fmt::Debug for ExternalFormatter {
impl ExternalFormatter {
/// Create an [`ExternalFormatter`] from JS callbacks.
///
/// The ThreadsafeFunctions are wrapped in `Arc<Mutex<Option<...>>>` to allow
/// The ThreadsafeFunctions are wrapped in `Arc<RwLock<Option<...>>>` to allow
/// explicit cleanup via the `cleanup()` method. This prevents use-after-free
/// crashes on Node.js exit when V8 cleans up global handles.
pub fn new(
Expand Down Expand Up @@ -351,7 +351,7 @@ fn wrap_format_embedded(
let status = cb.call_async(FnArgs::from((options, code.to_string()))).await;
match status {
Ok(promise) => match promise.await {
Ok(formatted_code) => Ok(formatted_code),
Ok(result) => parse_embedded_callback_result(result),
Err(err) => Err(err.reason.clone()),
},
Err(err) => Err(err.reason.clone()),
Expand All @@ -362,6 +362,39 @@ fn wrap_format_embedded(
})
}

/// Parse embedded formatter callback result.
///
/// New callback contract:
/// - `{ ok: true, code: string }` on success
/// - `{ ok: false, error: string }` on recoverable formatting failure
///
/// Legacy fallback:
/// - plain `string` is also accepted as success.
fn parse_embedded_callback_result(value: Value) -> Result<String, String> {
let invalid_response =
|| "Invalid embedded formatter response (expected `{ ok, code|error }`)".to_string();

match value {
Value::String(code) => Ok(code),
Value::Object(map) => {
let Some(ok) = map.get("ok").and_then(Value::as_bool) else {
return Err(invalid_response());
};
if ok {
map.get("code")
.and_then(Value::as_str)
.map(ToString::to_string)
.ok_or_else(invalid_response)
} else {
let err =
map.get("error").and_then(Value::as_str).unwrap_or("Unknown embedded error");
Err(err.to_string())
}
}
_ => Err(invalid_response()),
}
}

/// Wrap JS `formatFile` callback as a normal Rust function.
/// The `options` Value is received with `parser` and `filepath` already set by the caller.
fn wrap_format_file(
Expand Down
12 changes: 9 additions & 3 deletions apps/oxfmt/src/main_napi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ pub async fn run_cli(
args: Vec<String>,
#[napi(ts_arg_type = "(numThreads: number) => Promise<string[]>")]
init_external_formatter_cb: JsInitExternalFormatterCb,
#[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
#[napi(
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>"
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Rust side now accepts a legacy plain string result for backward compatibility (parse_embedded_callback_result), but this ts_arg_type only allows the wrapped { ok, code?, error? } object. If backward compatibility is intended, update the TypeScript callback type to include Promise<string | { ok: boolean; code?: string; error?: string }>; otherwise consider removing the legacy parsing path to keep runtime and types aligned.

Suggested change
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>"
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string | { ok: boolean; code?: string; error?: string }>"

Copilot uses AI. Check for mistakes.
)]
format_embedded_cb: JsFormatEmbeddedCb,
#[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
format_file_cb: JsFormatFileCb,
Expand Down Expand Up @@ -140,7 +142,9 @@ pub async fn format(
options: Option<Value>,
#[napi(ts_arg_type = "(numThreads: number) => Promise<string[]>")]
init_external_formatter_cb: JsInitExternalFormatterCb,
#[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
#[napi(
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>"
)]
Comment on lines +145 to +147
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: the Rust parser accepts legacy string results, but the generated TS arg type only permits the wrapped object. Consider widening this callback type to Promise<string | { ok: boolean; code?: string; error?: string }> (or remove the legacy parsing path if not supported).

Copilot uses AI. Check for mistakes.
format_embedded_cb: JsFormatEmbeddedCb,
#[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
format_file_cb: JsFormatFileCb,
Expand Down Expand Up @@ -176,7 +180,9 @@ pub async fn js_text_to_doc(
parent_context: String,
#[napi(ts_arg_type = "(numThreads: number) => Promise<string[]>")]
init_external_formatter_cb: JsInitExternalFormatterCb,
#[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
#[napi(
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>"
Copy link

Copilot AI Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: this callback ts_arg_type doesn’t reflect that Rust still accepts a legacy string response. Either widen the TS type to include string or drop the legacy support so type-level and runtime contracts match.

Suggested change
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<{ ok: boolean; code?: string; error?: string }>"
ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string | { ok: boolean; code?: string; error?: string }>"

Copilot uses AI. Check for mistakes.
)]
format_embedded_cb: JsFormatEmbeddedCb,
#[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
format_file_cb: JsFormatFileCb,
Expand Down
Loading