mirror of
https://github.com/oxc-project/oxc.git
synced 2026-11-04 10:29:58 +00:00
fix(react-compiler): preserve warning bailouts (#24935)
Preserve warning-level React Compiler diagnostics instead of wrapping pipeline bailouts in a synthetic fatal error. This keeps expected incompatible-library and Todo bailouts non-fatal so sibling functions can still compile. Adds regression coverage and refreshes the affected snapshots. Validated with `just ready`. AI-assisted: Codex was used to investigate, implement, and validate this change.
This commit is contained in:
12 files changed
+55
-56
No files matched your search
@@ -122,30 +122,6 @@ pub fn has_critical_errors(diagnostics: &[OxcDiagnostic]) -> bool {
|
||||
.any(|d| d.severity == Severity::Error && !ErrorCategory::PreserveManualMemo.matches(d))
|
||||
}
|
||||
|
||||
/// Format a thrown diagnostic as a string matching the TS
|
||||
/// `CompilerError.toString()` output, used for the `data` field of
|
||||
/// `CompileUnexpectedThrow` events: `"<Heading>: <reason>. <description>."`.
|
||||
/// The reason is the message minus the deterministic prefix added by
|
||||
/// [`ErrorCategory::diagnostic`].
|
||||
pub fn to_string_for_event(diagnostic: &OxcDiagnostic) -> String {
|
||||
let category = ErrorCategory::of(diagnostic);
|
||||
let heading = match category {
|
||||
Some("IncompatibleLibrary" | "PreserveManualMemo" | "UnsupportedSyntax") => {
|
||||
"Compilation Skipped"
|
||||
}
|
||||
Some(heading @ ("Invariant" | "Todo")) => heading,
|
||||
_ => "Error",
|
||||
};
|
||||
let reason = category
|
||||
.and_then(|c| diagnostic.message.strip_prefix(&format!("[ReactCompiler] {c}: ")))
|
||||
.unwrap_or(&diagnostic.message);
|
||||
let mut buf = format!("{heading}: {reason}");
|
||||
if let Some(help) = &diagnostic.help {
|
||||
buf.push_str(&format!(". {help}."));
|
||||
}
|
||||
buf
|
||||
}
|
||||
|
||||
/// Owned copy of a diagnostic for the log accumulator, labelling the enclosing
|
||||
/// function (`fn_span`) when the diagnostic carries no location of its own.
|
||||
#[cold]
|
||||
|
||||
@@ -10,9 +10,8 @@
|
||||
|
||||
use oxc_allocator::GetAllocator;
|
||||
use oxc_diagnostics::{Diagnostics, OxcDiagnostic};
|
||||
use oxc_span::Span;
|
||||
|
||||
use crate::diagnostics::{ErrorCategory, to_string_for_event};
|
||||
use crate::diagnostics::ErrorCategory;
|
||||
use crate::react_compiler_hir::ReactFunctionType;
|
||||
use crate::react_compiler_hir::environment::Environment;
|
||||
use crate::react_compiler_hir::environment::OutputMode;
|
||||
@@ -91,9 +90,6 @@ use crate::options::CompilerOutputMode;
|
||||
/// Run the compilation pipeline on a single function.
|
||||
///
|
||||
/// On failure, returns the diagnostics of the failed compilation attempt.
|
||||
/// An error thrown by a pass (in TS: an exception escaping the pass) that is
|
||||
/// not an Invariant additionally surfaces a `CompileUnexpectedThrow`
|
||||
/// diagnostic, matching TS `tryCompileFunction`'s catch block.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub fn compile_fn<'a>(
|
||||
ast: &oxc_ast::builder::AstBuilder<'a>,
|
||||
@@ -103,32 +99,19 @@ pub fn compile_fn<'a>(
|
||||
mode: CompilerOutputMode,
|
||||
env_config: &EnvironmentConfig,
|
||||
context: &mut ProgramContext<'a>,
|
||||
fn_span: Option<Span>,
|
||||
) -> Result<Option<CodegenFunction<'a>>, Diagnostics> {
|
||||
match run_pipeline(ast, func, scope, fn_type, mode, env_config, context) {
|
||||
Ok(result) => result,
|
||||
Err(thrown) => {
|
||||
if !ErrorCategory::Invariant.matches(&thrown) {
|
||||
let mut diagnostic = OxcDiagnostic::error(format!(
|
||||
"[ReactCompiler] Unexpected error: {}",
|
||||
to_string_for_event(&thrown)
|
||||
));
|
||||
if let Some(span) = fn_span {
|
||||
diagnostic = diagnostic.with_label(span);
|
||||
}
|
||||
context.diagnostics.push(diagnostic);
|
||||
}
|
||||
Err(Diagnostics::from(thrown))
|
||||
}
|
||||
Err(diagnostic) => Err(Diagnostics::from(diagnostic)),
|
||||
}
|
||||
}
|
||||
|
||||
/// The pass pipeline: creates an Environment, runs BuildHIR (lowering), the
|
||||
/// HIR/reactive-scope passes, and codegen.
|
||||
///
|
||||
/// `Err(OxcDiagnostic)` is an error thrown by a pass (a TS exception);
|
||||
/// `Err(OxcDiagnostic)` is a diagnostic that immediately bails out of a pass.
|
||||
/// Invariant and end-of-pipeline accumulated errors return as
|
||||
/// `Ok(Err(diagnostics))` since they must not surface `CompileUnexpectedThrow`.
|
||||
/// `Ok(Err(diagnostics))`.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn run_pipeline<'a>(
|
||||
ast: &oxc_ast::builder::AstBuilder<'a>,
|
||||
|
||||
@@ -1056,8 +1056,7 @@ fn try_compile_function<'a>(
|
||||
env_config: &EnvironmentConfig,
|
||||
context: &mut ProgramContext<'a>,
|
||||
) -> Result<Option<CodegenFunction<'a>>, Diagnostics> {
|
||||
// Check for suppressions that affect this function. Suppression errors are
|
||||
// returned (not thrown), so they do NOT trigger CompileUnexpectedThrow.
|
||||
// Check for suppressions that affect this function before entering the pipeline.
|
||||
if let (Some(start), Some(end)) = (source.fn_start, source.fn_end) {
|
||||
let affecting = filter_suppressions_that_affect_function(&context.suppressions, start, end);
|
||||
if !affecting.is_empty() {
|
||||
@@ -1075,7 +1074,6 @@ fn try_compile_function<'a>(
|
||||
output_mode,
|
||||
env_config,
|
||||
context,
|
||||
source.fn_ast_span,
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.dont-hoist-inline-reference
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Todo: [hoisting] EnterSSA: Expected identifier to be defined before being used. Identifier x$1 is undefined.", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(41), length: 62 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Todo: [hoisting] EnterSSA: Expected identifier to be defined before being used", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(67), length: 22 }, primary: false }]), help: Some("Identifier x$1 is undefined"), note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
-1
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.invalid-known-incompatible-
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Compilation Skipped: Use of incompatible library. This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized.", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(71), length: 87 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] IncompatibleLibrary: Use of incompatible library", labels: One([LabeledSpan { label: Some("useKnownIncompatible is known to be incompatible"), span: SourceSpan { offset: SourceOffset(109), length: 17 }, primary: false }]), help: Some("This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized"), note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
-1
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.invalid-known-incompatible-
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Compilation Skipped: Use of incompatible library. This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized.", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(82), length: 119 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] IncompatibleLibrary: Use of incompatible library", labels: One([LabeledSpan { label: Some("useKnownIncompatibleIndirect returns an incompatible() function that is known incompatible"), span: SourceSpan { offset: SourceOffset(177), length: 12 }, primary: false }]), help: Some("This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized"), note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.invalid-known-incompatible-
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Compilation Skipped: Use of incompatible library. This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized.", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(74), length: 90 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] IncompatibleLibrary: Use of incompatible library", labels: One([LabeledSpan { label: Some("useKnownIncompatible is known to be incompatible"), span: SourceSpan { offset: SourceOffset(112), length: 20 }, primary: false }]), help: Some("This API returns functions which cannot be memoized without leading to stale UI. To prevent this, by default React Compiler will skip memoizing this component/hook. However, you may see issues if values from this API are passed to other components/hooks that are memoized"), note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.todo-functiondecl-hoisting.
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Todo: [PruneHoistedContexts] Rewrite hoisted function references", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(330), length: 164 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Todo: [PruneHoistedContexts] Rewrite hoisted function references", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(371), length: 3 }, primary: false }]), help: None, note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
-1
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.todo-hook-call-spreads-muta
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Todo: Support spread syntax for hook arguments", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(45), length: 120 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Todo: Support spread syntax for hook arguments", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(147), length: 14 }, primary: false }]), help: None, note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
-1
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.todo-rust-as-expression-ass
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Todo: [FindContextIdentifiers] Cannot handle Object destructuring assignment target TSAsExpression", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(0), length: 137 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Todo: [FindContextIdentifiers] Cannot handle Object destructuring assignment target TSAsExpression", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(52), length: 32 }, primary: false }]), help: None, note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
@@ -4,7 +4,6 @@ input_file: crates/oxc_react_compiler/fixtures/error.todo-valid-functiondecl-hoi
|
||||
---
|
||||
Diagnostics:
|
||||
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Unexpected error: Todo: [PruneHoistedContexts] Rewrite hoisted function references", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(307), length: 180 }, primary: false }]), help: None, note: None, severity: Error, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
OxcDiagnostic { inner: OxcDiagnosticInner { message: "[ReactCompiler] Todo: [PruneHoistedContexts] Rewrite hoisted function references", labels: One([LabeledSpan { label: None, span: SourceSpan { offset: SourceOffset(369), length: 3 }, primary: false }]), help: None, note: None, severity: Warning, code: OxcCode { scope: None, number: None }, url: None } }
|
||||
|
||||
No changes.
|
||||
@@ -447,6 +447,56 @@ fn diagnostics_preserve_compiler_severity() {
|
||||
);
|
||||
}
|
||||
|
||||
/// A warning-level function bail-out must not be promoted to a fatal error:
|
||||
/// sibling functions should still compile and downstream transforms should run.
|
||||
#[test]
|
||||
fn incompatible_library_bailout_remains_a_warning() {
|
||||
let source = "\
|
||||
import { useReactTable } from '@tanstack/react-table';\n\
|
||||
function Table() {\n\
|
||||
const table = useReactTable({});\n\
|
||||
return <div>{table}</div>;\n\
|
||||
}\n\
|
||||
export function Component(props: { text: string }) {\n\
|
||||
return <span>{props.text}</span>;\n\
|
||||
}\n";
|
||||
|
||||
let allocator = Allocator::default();
|
||||
let (program, result) = transform_source(source, SourceType::tsx(), &allocator, options());
|
||||
|
||||
assert!(result.changed, "the unaffected component should compile");
|
||||
assert!(
|
||||
result.diagnostics.has_warnings(),
|
||||
"the incompatible library should report a warning: {:?}",
|
||||
result.diagnostics
|
||||
);
|
||||
assert!(
|
||||
!result.diagnostics.has_errors(),
|
||||
"a warning-level function bail-out must not become fatal: {:?}",
|
||||
result.diagnostics
|
||||
);
|
||||
assert_eq!(result.diagnostics.len(), 1);
|
||||
assert!(
|
||||
result.diagnostics[0].message.contains("[ReactCompiler] IncompatibleLibrary"),
|
||||
"expected the original compiler diagnostic: {:?}",
|
||||
result.diagnostics
|
||||
);
|
||||
assert!(
|
||||
result
|
||||
.diagnostics
|
||||
.iter()
|
||||
.all(|diagnostic| !diagnostic.message.contains("Unexpected error")),
|
||||
"the bail-out must not create a synthetic error: {:?}",
|
||||
result.diagnostics
|
||||
);
|
||||
|
||||
let output = Codegen::new().build(&program).code;
|
||||
assert!(
|
||||
output.contains("react/compiler-runtime"),
|
||||
"expected the unaffected component to be transformed:\n{output}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The transform only rewrites the compiled functions, so top-level comments
|
||||
/// survive; comments inside a compiled function are pruned because their anchor no
|
||||
/// longer resolves to a statement (see `prune_inner_comments`).
|
||||
|
||||
Reference in new issue
Block a user