diff --git a/compiler/Cargo.lock b/compiler/Cargo.lock index 14273c9c43c4..cd8be675a22b 100644 --- a/compiler/Cargo.lock +++ b/compiler/Cargo.lock @@ -257,6 +257,7 @@ name = "react_compiler_ast" version = "0.1.0" dependencies = [ "indexmap", + "react_compiler_diagnostics", "serde", "serde-transcode", "serde_json", diff --git a/compiler/crates/react_compiler/src/entrypoint/imports.rs b/compiler/crates/react_compiler/src/entrypoint/imports.rs index ab91b764895f..a495686ae565 100644 --- a/compiler/crates/react_compiler/src/entrypoint/imports.rs +++ b/compiler/crates/react_compiler/src/entrypoint/imports.rs @@ -279,7 +279,12 @@ pub fn validate_restricted_imports( for stmt in &program.body { if let Statement::ImportDeclaration(import) = stmt { - if restricted.contains(import.source.value.as_str()) { + if import + .source + .value + .as_str() + .is_some_and(|v| restricted.contains(v)) + { let mut detail = CompilerErrorDetail::new( ErrorCategory::Todo, "Bailing out due to blocklisted import", @@ -328,7 +333,7 @@ pub fn add_imports_to_program(program: &mut Program, context: &ProgramContext) { .filter_map(|(idx, stmt)| { if let Statement::ImportDeclaration(import) = stmt { if is_non_namespaced_import(import) { - return Some((import.source.value.clone(), idx)); + return Some((import.source.value.to_marker_string(), idx)); } } None @@ -363,7 +368,7 @@ pub fn add_imports_to_program(program: &mut Program, context: &ProgramContext) { specifiers: import_specifiers, source: StringLiteral { base: BaseNode::typed("StringLiteral"), - value: module_name.clone(), + value: module_name.clone().into(), }, import_kind: None, assertions: None, @@ -420,7 +425,7 @@ pub fn add_imports_to_program(program: &mut Program, context: &ProgramContext) { })), arguments: vec![Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: module_name.clone(), + value: module_name.clone().into(), })], type_parameters: None, type_arguments: None, diff --git a/compiler/crates/react_compiler/src/entrypoint/program.rs b/compiler/crates/react_compiler/src/entrypoint/program.rs index 8e6a16c20941..c01b7b90d7d5 100644 --- a/compiler/crates/react_compiler/src/entrypoint/program.rs +++ b/compiler/crates/react_compiler/src/entrypoint/program.rs @@ -1651,10 +1651,10 @@ fn has_memo_cache_function_import(program: &Program, module_name: &str) -> bool for specifier in &import.specifiers { if let ImportSpecifier::ImportSpecifier(data) = specifier { let imported_name = match &data.imported { - ModuleExportName::Identifier(id) => &id.name, - ModuleExportName::StringLiteral(s) => &s.value, + ModuleExportName::Identifier(id) => Some(id.name.as_str()), + ModuleExportName::StringLiteral(s) => s.value.as_str(), }; - if imported_name == "c" { + if imported_name == Some("c") { return true; } } diff --git a/compiler/crates/react_compiler_ast/Cargo.toml b/compiler/crates/react_compiler_ast/Cargo.toml index 426f00f924a5..a5b504b0de80 100644 --- a/compiler/crates/react_compiler_ast/Cargo.toml +++ b/compiler/crates/react_compiler_ast/Cargo.toml @@ -4,6 +4,7 @@ version = "0.1.0" edition = "2024" [dependencies] +react_compiler_diagnostics = { path = "../react_compiler_diagnostics" } serde = { version = "1", features = ["derive"] } serde_json = { version = "1", features = ["raw_value", "unbounded_depth"] } serde-transcode = "1" diff --git a/compiler/crates/react_compiler_ast/src/literals.rs b/compiler/crates/react_compiler_ast/src/literals.rs index 356543d40965..c903d2223beb 100644 --- a/compiler/crates/react_compiler_ast/src/literals.rs +++ b/compiler/crates/react_compiler_ast/src/literals.rs @@ -1,3 +1,4 @@ +use react_compiler_diagnostics::JsString; use serde::{Deserialize, Serialize}; use crate::common::BaseNode; @@ -6,7 +7,8 @@ use crate::common::BaseNode; pub struct StringLiteral { #[serde(flatten)] pub base: BaseNode, - pub value: String, + /// JS string values may contain unpaired surrogates; see [`JsString`]. + pub value: JsString, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/compiler/crates/react_compiler_diagnostics/src/js_string.rs b/compiler/crates/react_compiler_diagnostics/src/js_string.rs new file mode 100644 index 000000000000..b6f8caa3ce8d --- /dev/null +++ b/compiler/crates/react_compiler_diagnostics/src/js_string.rs @@ -0,0 +1,335 @@ +//! A JavaScript string value. JS strings are sequences of UTF-16 code units +//! with no validity requirement, so a value can contain unpaired surrogate +//! halves that Rust's `String` cannot represent. `JsString` keeps the common +//! valid case as UTF-8 and falls back to code units only when the value is +//! ill-formed, so the compiler computes on true program values instead of +//! replacement characters or escape hatches. +//! +//! Wire format: the babel bridge transports lone surrogates as +//! `__SURROGATE_XXXX__` markers (see `sanitizeJsonSurrogates` in bridge.ts), +//! because serde_json can neither parse nor emit a lone `\uXXXX` escape. +//! Serde for `JsString` decodes and re-emits that marker form, which keeps the +//! JS side of the bridge unchanged. + +use std::fmt; + +use serde::Deserialize; +use serde::Serialize; + +/// Invariant: `Repr::Utf8` holds every well-formed value and `Repr::Wtf16` +/// only ill-formed ones (at least one unpaired surrogate). The derived +/// `PartialEq`/`Hash` are only sound under this invariant: a well-formed +/// value smuggled into `Wtf16` would compare unequal to its `Utf8` twin. The +/// representation is private so the invariant holds by construction; match on +/// [`JsString::as_ref`] to branch on well-formedness. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct JsString(Repr); + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +enum Repr { + /// A well-formed string (no unpaired surrogates), stored as UTF-8. + Utf8(String), + /// An ill-formed string, stored as UTF-16 code units. + Wtf16(Vec), +} + +/// Borrowed view of a [`JsString`] for callers that need to branch on +/// well-formedness. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum JsStringRef<'a> { + Utf8(&'a str), + Wtf16(&'a [u16]), +} + +impl JsString { + /// Build from UTF-16 code units, normalizing to UTF-8 when well-formed. + pub fn from_code_units(units: Vec) -> Self { + match String::from_utf16(&units) { + Ok(s) => JsString(Repr::Utf8(s)), + Err(_) => JsString(Repr::Wtf16(units)), + } + } + + pub fn as_ref(&self) -> JsStringRef<'_> { + match &self.0 { + Repr::Utf8(s) => JsStringRef::Utf8(s), + Repr::Wtf16(units) => JsStringRef::Wtf16(units), + } + } + + /// The UTF-8 view, when the value is well-formed. + pub fn as_str(&self) -> Option<&str> { + match &self.0 { + Repr::Utf8(s) => Some(s), + Repr::Wtf16(_) => None, + } + } + + pub fn code_units(&self) -> Vec { + match &self.0 { + Repr::Utf8(s) => s.encode_utf16().collect(), + Repr::Wtf16(units) => units.clone(), + } + } + + /// Length in UTF-16 code units (JS `String.prototype.length`). + pub fn len_utf16(&self) -> usize { + match &self.0 { + Repr::Utf8(s) => s.encode_utf16().count(), + Repr::Wtf16(units) => units.len(), + } + } + + /// The value with unpaired surrogates replaced by U+FFFD, for consumers + /// whose string type cannot represent ill-formed values. + pub fn to_string_lossy(&self) -> String { + match &self.0 { + Repr::Utf8(s) => s.clone(), + Repr::Wtf16(units) => String::from_utf16_lossy(units), + } + } + + /// Decode the bridge wire form: a UTF-8 string in which lone surrogates + /// appear as `__SURROGATE_XXXX__` markers (uppercase hex, mirroring what + /// `sanitizeJsonSurrogates` emits and `restoreJsonSurrogates` accepts). + /// + /// All scanning is byte-wise: a marker is 18 ASCII bytes, so byte-slice + /// comparisons cannot land on a UTF-8 char boundary the way `str` range + /// indexing can when multibyte text follows the prefix. + pub fn from_marker_string(s: &str) -> Self { + const PREFIX: &[u8] = b"__SURROGATE_"; + const MARKER_LEN: usize = 18; + if !s.contains("__SURROGATE_") { + return JsString(Repr::Utf8(s.to_string())); + } + let bytes = s.as_bytes(); + let mut units: Vec = Vec::with_capacity(s.len()); + let mut pos = 0; + let mut segment_start = 0; + while let Some(found) = s[pos..].find("__SURROGATE_") { + let idx = pos + found; + let tail = &bytes[idx..]; + let well_formed = tail.len() >= MARKER_LEN + && &tail[MARKER_LEN - 2..MARKER_LEN] == b"__" + && tail[PREFIX.len()..PREFIX.len() + 4] + .iter() + .all(|b| b.is_ascii_hexdigit() && !b.is_ascii_lowercase()); + if well_formed { + let hex = std::str::from_utf8(&tail[PREFIX.len()..PREFIX.len() + 4]) + .expect("ascii hex is valid utf8"); + let unit = u16::from_str_radix(hex, 16).expect("validated hex digits"); + units.extend(s[segment_start..idx].encode_utf16()); + units.push(unit); + pos = idx + MARKER_LEN; + segment_start = pos; + } else { + // Not a well-formed marker: keep the literal text and continue + // scanning after the prefix. + pos = idx + PREFIX.len(); + } + } + units.extend(s[segment_start..].encode_utf16()); + JsString::from_code_units(units) + } + + /// Encode to the bridge wire form (markers for unpaired surrogates). + pub fn to_marker_string(&self) -> String { + match &self.0 { + Repr::Utf8(s) => s.clone(), + Repr::Wtf16(units) => { + let mut out = String::with_capacity(units.len() * 2); + let mut iter = units.iter().copied().peekable(); + while let Some(unit) = iter.next() { + match unit { + 0xD800..=0xDBFF => { + if let Some(&next) = iter.peek() { + if (0xDC00..=0xDFFF).contains(&next) { + iter.next(); + let cp = 0x10000 + + ((unit as u32 - 0xD800) << 10) + + (next as u32 - 0xDC00); + out.push(char::from_u32(cp).expect("valid supplementary")); + continue; + } + } + out.push_str(&format!("__SURROGATE_{unit:04X}__")); + } + 0xDC00..=0xDFFF => { + out.push_str(&format!("__SURROGATE_{unit:04X}__")); + } + _ => { + out.push( + char::from_u32(unit as u32).expect("BMP non-surrogate is a char"), + ); + } + } + } + out + } + } + } + + /// Render as JS-source-style escaped text, matching the form TS's debug + /// printer produces via JSON.stringify: unpaired surrogates print as + /// lowercase `\udXXX` escapes inside the otherwise UTF-8 text. + pub fn to_escaped_string(&self) -> String { + match &self.0 { + Repr::Utf8(s) => s.clone(), + Repr::Wtf16(units) => { + let mut out = String::with_capacity(units.len() * 2); + let mut iter = units.iter().copied().peekable(); + while let Some(unit) = iter.next() { + match unit { + 0xD800..=0xDBFF => { + if let Some(&next) = iter.peek() { + if (0xDC00..=0xDFFF).contains(&next) { + iter.next(); + let cp = 0x10000 + + ((unit as u32 - 0xD800) << 10) + + (next as u32 - 0xDC00); + out.push(char::from_u32(cp).expect("valid supplementary")); + continue; + } + } + out.push_str(&format!("\\u{unit:04x}")); + } + 0xDC00..=0xDFFF => { + out.push_str(&format!("\\u{unit:04x}")); + } + _ => { + out.push( + char::from_u32(unit as u32).expect("BMP non-surrogate is a char"), + ); + } + } + } + out + } + } + } +} + +impl From for JsString { + fn from(s: String) -> Self { + // A Rust String is valid UTF-8 and so cannot contain an unpaired + // surrogate; constructing Utf8 directly preserves the invariant. + JsString(Repr::Utf8(s)) + } +} + +impl From<&str> for JsString { + fn from(s: &str) -> Self { + JsString(Repr::Utf8(s.to_string())) + } +} + +impl PartialEq for JsString { + fn eq(&self, other: &str) -> bool { + self.as_str() == Some(other) + } +} + +impl PartialEq<&str> for JsString { + fn eq(&self, other: &&str) -> bool { + self.as_str() == Some(*other) + } +} + +impl fmt::Display for JsString { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(&self.to_escaped_string()) + } +} + +impl Serialize for JsString { + fn serialize(&self, serializer: S) -> Result { + serializer.serialize_str(&self.to_marker_string()) + } +} + +impl<'de> Deserialize<'de> for JsString { + fn deserialize>(deserializer: D) -> Result { + let s = String::deserialize(deserializer)?; + Ok(JsString::from_marker_string(&s)) + } +} + +#[cfg(test)] +mod tests { + use super::JsString; + use super::JsStringRef; + + #[test] + fn as_ref_views_match_well_formedness() { + assert!(matches!( + JsString::from("plain").as_ref(), + JsStringRef::Utf8("plain") + )); + assert!(matches!( + JsString::from_code_units(vec![0xD83E]).as_ref(), + JsStringRef::Wtf16(&[0xD83E]) + )); + // Well-formed code units normalize to the Utf8 representation, so + // equal logical strings are equal values regardless of how they + // were constructed. + assert_eq!( + JsString::from_code_units("plain".encode_utf16().collect()), + JsString::from("plain") + ); + } + + #[test] + fn marker_round_trip_preserves_lone_surrogates() { + let js = JsString::from_marker_string("__SURROGATE_D83E__"); + assert_eq!(js.code_units(), vec![0xD83E]); + assert_eq!(js.to_marker_string(), "__SURROGATE_D83E__"); + assert_eq!(js.to_escaped_string(), "\\ud83e"); + } + + #[test] + fn paired_halves_render_as_the_supplementary_character() { + let js = JsString::from_code_units(vec![0xD83E, 0xDD21]); + assert_eq!(js.as_str(), Some("\u{1F921}")); + } + + #[test] + fn plain_strings_stay_utf8_and_compare_with_str() { + let js = JsString::from("use memo"); + assert!(js == "use memo"); + assert_eq!(js.to_marker_string(), "use memo"); + } + + #[test] + fn malformed_marker_text_is_kept_literally() { + let js = JsString::from_marker_string("__SURROGATE_XYZ__"); + assert_eq!(js.as_str(), Some("__SURROGATE_XYZ__")); + } + + #[test] + fn multibyte_text_after_marker_prefix_does_not_panic() { + let input = "__SURROGATE_\u{20AC}\u{20AC}"; + let js = JsString::from_marker_string(input); + assert_eq!(js.as_str(), Some(input)); + + let truncated = "__SURROGATE_D8"; + assert_eq!( + JsString::from_marker_string(truncated).as_str(), + Some(truncated) + ); + + let mixed = "a\u{20AC}__SURROGATE_D83E__b\u{20AC}"; + let js = JsString::from_marker_string(mixed); + let mut expected: Vec = "a\u{20AC}".encode_utf16().collect(); + expected.push(0xD83E); + expected.extend("b\u{20AC}".encode_utf16()); + assert_eq!(js.code_units(), expected); + } + + #[test] + fn lowercase_hex_markers_are_not_decoded() { + // The bridge emits uppercase hex only; lowercase marker-shaped text is + // user text and must survive verbatim. + let input = "__SURROGATE_d83e__"; + assert_eq!(JsString::from_marker_string(input).as_str(), Some(input)); + } +} diff --git a/compiler/crates/react_compiler_diagnostics/src/lib.rs b/compiler/crates/react_compiler_diagnostics/src/lib.rs index 16debb24ec7c..7bb3b63a88e3 100644 --- a/compiler/crates/react_compiler_diagnostics/src/lib.rs +++ b/compiler/crates/react_compiler_diagnostics/src/lib.rs @@ -1,4 +1,7 @@ pub mod code_frame; +pub mod js_string; + +pub use js_string::JsString; use serde::{Deserialize, Serialize}; diff --git a/compiler/crates/react_compiler_hir/src/lib.rs b/compiler/crates/react_compiler_hir/src/lib.rs index a23296c79e9a..07db0772561d 100644 --- a/compiler/crates/react_compiler_hir/src/lib.rs +++ b/compiler/crates/react_compiler_hir/src/lib.rs @@ -845,7 +845,7 @@ pub enum PrimitiveValue { Undefined, Boolean(bool), Number(FloatValue), - String(String), + String(react_compiler_diagnostics::JsString), } #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/compiler/crates/react_compiler_hir/src/print.rs b/compiler/crates/react_compiler_hir/src/print.rs index 8589391dc7b6..989efa91b789 100644 --- a/compiler/crates/react_compiler_hir/src/print.rs +++ b/compiler/crates/react_compiler_hir/src/print.rs @@ -80,7 +80,53 @@ pub fn format_primitive(prim: &crate::PrimitiveValue) -> String { crate::PrimitiveValue::Undefined => "undefined".to_string(), crate::PrimitiveValue::Boolean(b) => format!("{}", b), crate::PrimitiveValue::Number(n) => crate::format_js_number(n.value()), - crate::PrimitiveValue::String(s) => format_js_string(s), + crate::PrimitiveValue::String(s) => match s.as_str() { + Some(utf8) => format_js_string(utf8), + // Ill-formed strings: escape the well-formed segments exactly like + // format_js_string and render each unpaired surrogate as \uXXXX, + // matching what TS's JSON.stringify-based printer emits. + None => { + let mut result = String::new(); + result.push('"'); + let mut units = s.code_units().into_iter().peekable(); + while let Some(unit) = units.next() { + let is_lead = (0xD800..=0xDBFF).contains(&unit); + let is_trail = (0xDC00..=0xDFFF).contains(&unit); + if is_lead { + if let Some(&next) = units.peek() { + if (0xDC00..=0xDFFF).contains(&next) { + units.next(); + let cp = 0x10000 + + ((unit as u32 - 0xD800) << 10) + + (next as u32 - 0xDC00); + result.push(char::from_u32(cp).expect("valid supplementary")); + continue; + } + } + } + if is_lead || is_trail { + result.push_str(&format!("\\u{unit:04x}")); + continue; + } + let c = char::from_u32(unit as u32).expect("BMP non-surrogate is a char"); + match c { + '"' => result.push_str("\\\""), + '\\' => result.push_str("\\\\"), + '\n' => result.push_str("\\n"), + '\r' => result.push_str("\\r"), + '\t' => result.push_str("\\t"), + '\u{0008}' => result.push_str("\\b"), + '\u{000c}' => result.push_str("\\f"), + c if (c as u32) <= 0x1F => { + result.push_str(&format!("\\u{:04x}", c as u32)); + } + c => result.push(c), + } + } + result.push('"'); + result + } + }, } } diff --git a/compiler/crates/react_compiler_inference/src/memoize_fbt_and_macro_operands_in_same_scope.rs b/compiler/crates/react_compiler_inference/src/memoize_fbt_and_macro_operands_in_same_scope.rs index f63ca1c418ef..65969a902888 100644 --- a/compiler/crates/react_compiler_inference/src/memoize_fbt_and_macro_operands_in_same_scope.rs +++ b/compiler/crates/react_compiler_inference/src/memoize_fbt_and_macro_operands_in_same_scope.rs @@ -134,7 +134,9 @@ fn populate_macro_tags( value: PrimitiveValue::String(s), .. } => { - if let Some(macro_def) = macro_kinds.get(s.as_str()) { + if let Some(macro_def) = + s.as_str().and_then(|utf8| macro_kinds.get(utf8)) + { // We don't distinguish between tag names and strings, so record // all `fbt` string literals in case they are used as a jsx tag. macro_tags.insert(lvalue_id, macro_def.clone()); diff --git a/compiler/crates/react_compiler_lowering/src/build_hir.rs b/compiler/crates/react_compiler_lowering/src/build_hir.rs index ec605a0061ef..c3d3bd687632 100644 --- a/compiler/crates/react_compiler_lowering/src/build_hir.rs +++ b/compiler/crates/react_compiler_lowering/src/build_hir.rs @@ -6289,7 +6289,7 @@ fn lower_jsx_element_name( let place = lower_value_to_temporary( builder, InstructionValue::Primitive { - value: PrimitiveValue::String(tag), + value: PrimitiveValue::String(tag.into()), loc: loc.clone(), }, )?; @@ -6518,8 +6518,10 @@ fn lower_object_property_key( ) -> Result, CompilerError> { use react_compiler_ast::expressions::Expression; match key { + // Property keys stay String-typed; the marker wire form preserves the + // pre-JsString behavior for pathological surrogate keys end to end. Expression::StringLiteral(lit) => Ok(Some(ObjectPropertyKey::String { - name: lit.value.clone(), + name: lit.value.to_marker_string(), })), Expression::Identifier(ident) if !computed => Ok(Some(ObjectPropertyKey::Identifier { name: ident.name.clone(), diff --git a/compiler/crates/react_compiler_optimization/src/constant_propagation.rs b/compiler/crates/react_compiler_optimization/src/constant_propagation.rs index 65165f7e73b1..3b227b5b464e 100644 --- a/compiler/crates/react_compiler_optimization/src/constant_propagation.rs +++ b/compiler/crates/react_compiler_optimization/src/constant_propagation.rs @@ -26,6 +26,7 @@ use std::collections::HashMap; +use react_compiler_diagnostics::JsString; use react_compiler_hir::environment::Environment; use react_compiler_hir::{ BinaryOperator, BlockKind, FloatValue, FunctionId, GotoVariant, HirFunction, IdentifierId, @@ -303,10 +304,11 @@ fn evaluate_instruction( }) = prop_value { match prim { - PrimitiveValue::String(s) if is_valid_identifier(s) => { + PrimitiveValue::String(s) if s.as_str().is_some_and(is_valid_identifier) => { let object = object.clone(); let loc = *loc; - let new_property = PropertyLiteral::String(s.clone()); + let new_property = + PropertyLiteral::String(s.as_str().expect("guarded utf8").to_string()); func.instructions[instr_id.0 as usize].value = InstructionValue::PropertyLoad { object, @@ -345,11 +347,12 @@ fn evaluate_instruction( }) = prop_value { match prim { - PrimitiveValue::String(s) if is_valid_identifier(s) => { + PrimitiveValue::String(s) if s.as_str().is_some_and(is_valid_identifier) => { let object = object.clone(); let store_value = value.clone(); let loc = *loc; - let new_property = PropertyLiteral::String(s.clone()); + let new_property = + PropertyLiteral::String(s.as_str().expect("guarded utf8").to_string()); func.instructions[instr_id.0 as usize].value = InstructionValue::PropertyStore { object, @@ -534,7 +537,7 @@ fn evaluate_instruction( if let PropertyLiteral::String(prop_name) = property { if prop_name == "length" { // Use UTF-16 code unit count to match JS .length semantics - let len = s.encode_utf16().count() as f64; + let len = s.len_utf16() as f64; let loc = *loc; let result = Constant::Primitive { value: PrimitiveValue::Number(FloatValue::new(len)), @@ -567,11 +570,11 @@ fn evaluate_instruction( } let loc = *loc; let result = Constant::Primitive { - value: PrimitiveValue::String(result_string.clone()), + value: PrimitiveValue::String(JsString::from_marker_string(&result_string)), loc, }; func.instructions[instr_id.0 as usize].value = InstructionValue::Primitive { - value: PrimitiveValue::String(result_string), + value: PrimitiveValue::String(JsString::from_marker_string(&result_string)), loc, }; return Some(result); @@ -600,7 +603,7 @@ fn evaluate_instruction( PrimitiveValue::Null => "null".to_string(), PrimitiveValue::Boolean(b) => b.to_string(), PrimitiveValue::Number(n) => format_js_number(n.value()), - PrimitiveValue::String(s) => s.clone(), + PrimitiveValue::String(s) => s.to_marker_string(), // TS rejects undefined subexpression values PrimitiveValue::Undefined => return None, }; @@ -617,11 +620,11 @@ fn evaluate_instruction( let loc = *loc; let result = Constant::Primitive { - value: PrimitiveValue::String(result_string.clone()), + value: PrimitiveValue::String(JsString::from_marker_string(&result_string)), loc, }; func.instructions[instr_id.0 as usize].value = InstructionValue::Primitive { - value: PrimitiveValue::String(result_string), + value: PrimitiveValue::String(JsString::from_marker_string(&result_string)), loc, }; Some(result) @@ -847,7 +850,7 @@ fn is_truthy(value: &PrimitiveValue) -> bool { let v = n.value(); v != 0.0 && !v.is_nan() } - PrimitiveValue::String(s) => !s.is_empty(), + PrimitiveValue::String(s) => s.len_utf16() != 0, } } @@ -866,9 +869,13 @@ fn evaluate_binary_op( FloatValue::new(l.value() + r.value()), )), (PrimitiveValue::String(l), PrimitiveValue::String(r)) => { - let mut s = l.clone(); - s.push_str(r); - Some(PrimitiveValue::String(s)) + // Concatenate as code units: JS `+` can pair up surrogate + // halves split across the operands. + let mut units = l.code_units(); + units.extend(r.code_units()); + Some(PrimitiveValue::String( + react_compiler_diagnostics::JsString::from_code_units(units), + )) } _ => None, }, @@ -1056,8 +1063,12 @@ fn js_abstract_equal(lhs: &PrimitiveValue, rhs: &PrimitiveValue) -> bool { // Cross-type coercions for primitives (PrimitiveValue::Number(n), PrimitiveValue::String(s)) | (PrimitiveValue::String(s), PrimitiveValue::Number(n)) => { - // String is coerced to number using JS ToNumber semantics - let sv = js_to_number(s); + // String is coerced to number using JS ToNumber semantics. + // Ill-formed strings coerce to NaN, like any non-numeric text. + let sv = match s.as_str() { + Some(utf8) => js_to_number(utf8), + None => f64::NAN, + }; let nv = n.value(); if nv.is_nan() || sv.is_nan() { false diff --git a/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs b/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs index 078db1f7a3b2..55264109fe4d 100644 --- a/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs +++ b/compiler/crates/react_compiler_reactive_scopes/src/codegen_reactive_function.rs @@ -282,7 +282,7 @@ pub fn codegen_function( })), right: Box::new(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: hash.clone(), + value: hash.clone().into(), })), })), consequent: Box::new(Statement::BlockStatement(BlockStatement { @@ -382,7 +382,7 @@ pub fn codegen_function( arguments: vec![Expression::StringLiteral( StringLiteral { base: BaseNode::typed("StringLiteral"), - value: MEMO_CACHE_SENTINEL.to_string(), + value: MEMO_CACHE_SENTINEL.to_string().into(), }, )], type_parameters: None, @@ -421,7 +421,7 @@ pub fn codegen_function( )), right: Box::new(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: hash.clone(), + value: hash.clone().into(), })), }, )), @@ -493,11 +493,11 @@ pub fn codegen_function( arguments: vec![ Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: fn_name_str.to_string(), + value: fn_name_str.to_string().into(), }), Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: filename_str.to_string(), + value: filename_str.to_string().into(), }), ], type_parameters: None, @@ -2010,7 +2010,7 @@ fn codegen_instruction_value( })?; expressions.push(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: format!("TODO handle declaration"), + value: format!("TODO handle declaration").into(), })); } _ => { @@ -2025,7 +2025,7 @@ fn codegen_instruction_value( })?; expressions.push(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: format!("TODO handle statement"), + value: format!("TODO handle statement").into(), })); } } @@ -2714,7 +2714,7 @@ fn codegen_function_expression( base: BaseNode::typed("ObjectProperty"), key: Box::new(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: hint.clone(), + value: hint.clone().into(), })), value: Box::new(value), computed: false, @@ -2726,7 +2726,7 @@ fn codegen_function_expression( })), property: Box::new(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: hint.clone(), + value: hint.clone().into(), })), computed: true, }); @@ -2848,7 +2848,7 @@ fn codegen_object_property_key( match key { ObjectPropertyKey::String { name } => Ok(Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: name.clone(), + value: name.clone().into(), })), ObjectPropertyKey::Identifier { name } => Ok(Expression::Identifier(make_identifier(name))), ObjectPropertyKey::Computed { name } => { @@ -2892,7 +2892,7 @@ fn codegen_jsx_expression( JsxTag::Builtin(builtin) => ( Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: builtin.name.clone(), + value: builtin.name.clone().into(), }), None, ), @@ -2901,7 +2901,9 @@ fn codegen_jsx_expression( let jsx_tag = expression_to_jsx_tag(&tag_value, jsx_tag_loc(tag))?; let is_fbt_tag = if let Expression::StringLiteral(ref s) = tag_value { - SINGLE_CHILD_FBT_TAGS.contains(&s.value.as_str()) + s.value + .as_str() + .is_some_and(|v| SINGLE_CHILD_FBT_TAGS.contains(&v)) } else { false }; @@ -3002,7 +3004,7 @@ fn codegen_jsx_attribute( let inner_value = codegen_place_to_expression(cx, place)?; let attr_value = match &inner_value { Expression::StringLiteral(s) => { - if string_requires_expr_container(&s.value) + if string_requires_expr_container(&s.value.to_marker_string()) && !cx.fbt_operands.contains(&place.identifier) { Some(JSXAttributeValue::JSXExpressionContainer( @@ -3065,7 +3067,7 @@ fn codegen_jsx_element(cx: &mut Context, place: &Place) -> Result { - if s.value.contains(':') { - let parts: Vec<&str> = s.value.splitn(2, ':').collect(); + // JSX tag names are identifier-shaped; the marker form preserves + // the pre-JsString behavior for pathological values. + let tag_text = s.value.to_marker_string(); + if tag_text.contains(':') { + let parts: Vec<&str> = tag_text.splitn(2, ':').collect(); Ok(JSXElementName::JSXNamespacedName(JSXNamespacedName { base: base_node_with_loc("JSXNamespacedName", loc), namespace: JSXIdentifier { @@ -3137,7 +3142,7 @@ fn expression_to_jsx_tag( } else { Ok(JSXElementName::JSXIdentifier(JSXIdentifier { base: base_node_with_loc("JSXIdentifier", loc), - name: s.value.clone(), + name: tag_text, })) } } @@ -3746,7 +3751,7 @@ fn symbol_for(name: &str) -> Expression { })), arguments: vec![Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: name.to_string(), + value: name.to_string().into(), })], type_parameters: None, type_arguments: None, @@ -3824,7 +3829,7 @@ fn convert_value_to_expression(value: ExpressionOrJsxText) -> Expression { ExpressionOrJsxText::Expression(e) => e, ExpressionOrJsxText::JsxText(text) => Expression::StringLiteral(StringLiteral { base: BaseNode::typed("StringLiteral"), - value: text.value, + value: text.value.into(), }), } } diff --git a/compiler/crates/react_compiler_reactive_scopes/src/propagate_early_returns.rs b/compiler/crates/react_compiler_reactive_scopes/src/propagate_early_returns.rs index 235fb8c1da79..ea492192f41a 100644 --- a/compiler/crates/react_compiler_reactive_scopes/src/propagate_early_returns.rs +++ b/compiler/crates/react_compiler_reactive_scopes/src/propagate_early_returns.rs @@ -264,7 +264,7 @@ fn apply_early_return_to_scope( loc: None, // GeneratedSource }), value: ReactiveValue::Instruction(InstructionValue::Primitive { - value: PrimitiveValue::String(EARLY_RETURN_SENTINEL.to_string()), + value: PrimitiveValue::String(EARLY_RETURN_SENTINEL.into()), loc, }), effects: None, diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 95633283b2c7..473b86aad2a8 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -2085,6 +2085,55 @@ describe('ReactFlight', () => { ]); }); + it('should serialize an own __proto__ property nested among siblings without disturbing them', async () => { + // `__proto__` here is a real own enumerable data property (not the + // prototype). It sits between sibling keys and holds an object value, which + // is the case most likely to regress if the serializer used a plain + // `obj.__proto__ = value` assignment: that would hit the prototype setter, + // dropping the key and mutating the holder's prototype instead. + const value = {a: 1}; + Object.defineProperty(value, '__proto__', { + value: {nested: true}, + enumerable: true, + writable: true, + configurable: true, + }); + value.b = 2; + + const transport = ReactNoopFlightServer.render(value); + assertConsoleErrorDev([ + 'Expected not to serialize an object with own property `__proto__`. ' + + 'When parsed this property will be omitted.\n' + + ' {a: 1, __proto__: {nested: true}, b: 2}\n' + + ' ^^^^^^^^^^^^^^', + ]); + + const decoder = new TextDecoder(); + const payload = transport + .map(chunk => (typeof chunk === 'string' ? chunk : decoder.decode(chunk))) + .join(''); + // The legacy key is serialized as ordinary data, in source order, with its + // object value intact and without clobbering its sibling properties. + expect(payload).toContain('"a":1,"__proto__":{"nested":true},"b":2'); + + const model = await ReactNoopFlightClient.read(transport); + assertConsoleErrorDev([ + 'Expected not to serialize an object with own property `__proto__`. ' + + 'When parsed this property will be omitted.\n' + + ' {a: 1, __proto__: {nested: true}, b: 2}\n' + + ' ^^^^^^^^^^^^^^\n' + + ' in (at **)', + ]); + // On the client the legacy key is omitted, but its siblings survive intact + // and the holder's prototype is untouched. + expect(Object.prototype.hasOwnProperty.call(model, '__proto__')).toBe( + false, + ); + expect(Object.getPrototypeOf(model)).toBe(Object.prototype); + expect(model.a).toBe(1); + expect(model.b).toBe(2); + }); + it('should NOT warn in DEV for key getters', () => { const transport = ReactNoopFlightServer.render(
); ReactNoopFlightClient.read(transport); diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 9e46915fee44..189059f0b7c3 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -530,7 +530,6 @@ type Task = { status: 0 | 1 | 3 | 4 | 5, model: ReactClientValue, ping: () => void, - toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, keyPath: ReactKey, // parent server component keys implicitSlot: boolean, // true if the root server component of this sequence had a null key formatContext: FormatContext, // an approximate parent context from host components @@ -2761,55 +2760,6 @@ function createTask( implicitSlot, formatContext: formatContext, ping: () => pingTask(request, task), - toJSON: function ( - this: - | {+[key: string | number]: ReactClientValue} - | $ReadOnlyArray, - parentPropertyName: string, - value: ReactClientValue, - ): ReactJSONValue { - const parent = this; - // Make sure that `parent[parentPropertyName]` wasn't JSONified before `value` was passed to us - if (__DEV__) { - // $FlowFixMe[incompatible-use] - const originalValue = parent[parentPropertyName]; - if ( - typeof originalValue === 'object' && - originalValue !== value && - !(originalValue instanceof Date) - ) { - // Call with the server component as the currently rendering component - // for context. - callWithDebugContextInDEV(request, task, () => { - if (objectName(originalValue) !== 'Object') { - const jsxParentType = jsxChildrenParents.get(parent); - if (typeof jsxParentType === 'string') { - console.error( - '%s objects cannot be rendered as text children. Try formatting it using toString().%s', - objectName(originalValue), - describeObjectForErrorMessage(parent, parentPropertyName), - ); - } else { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - '%s objects are not supported.%s', - objectName(originalValue), - describeObjectForErrorMessage(parent, parentPropertyName), - ); - } - } else { - console.error( - 'Only plain objects can be passed to Client Components from Server Components. ' + - 'Objects with toJSON methods are not supported. Convert it manually ' + - 'to a simple value before passing it to props.%s', - describeObjectForErrorMessage(parent, parentPropertyName), - ); - } - }); - } - } - return renderModel(request, task, parent, parentPropertyName, value); - }, thenableState: null, } as Omit< Task, @@ -2837,6 +2787,117 @@ function createTask( return task; } +function resolveModel( + request: Request, + task: Task, + parent: + | {+[key: string | number]: ReactClientValue} + | $ReadOnlyArray, + parentPropertyName: string, + value: ReactClientValue, +): ReactJSONValue { + // Replicate JSON.stringify's toJSON semantics: if the value has a toJSON + // method, call it first. In practice this only matters for Date objects whose + // toJSON calls toISOString. Custom toJSON objects are not supported and will + // trigger a DEV warning below. + let jsonValue: ReactClientValue = value; + if ( + value !== null && + typeof value === 'object' && + // $FlowFixMe[method-unbinding] + typeof value.toJSON === 'function' + ) { + // $FlowFixMe[incompatible-use] + jsonValue = value.toJSON(parentPropertyName); + } + + if (__DEV__) { + // $FlowFixMe[incompatible-use] + const originalValue = parent[parentPropertyName]; + if ( + typeof originalValue === 'object' && + originalValue !== jsonValue && + !(originalValue instanceof Date) + ) { + // Call with the server component as the currently rendering component + // for context. + callWithDebugContextInDEV(request, task, () => { + if (objectName(originalValue) !== 'Object') { + const jsxParentType = jsxChildrenParents.get(parent); + if (typeof jsxParentType === 'string') { + console.error( + '%s objects cannot be rendered as text children. Try formatting it using toString().%s', + objectName(originalValue), + describeObjectForErrorMessage(parent, parentPropertyName), + ); + } else { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + '%s objects are not supported.%s', + objectName(originalValue), + describeObjectForErrorMessage(parent, parentPropertyName), + ); + } + } else { + console.error( + 'Only plain objects can be passed to Client Components from Server Components. ' + + 'Objects with toJSON methods are not supported. Convert it manually ' + + 'to a simple value before passing it to props.%s', + describeObjectForErrorMessage(parent, parentPropertyName), + ); + } + }); + } + } + + const rendered = renderModel( + request, + task, + parent, + parentPropertyName, + jsonValue, + ); + + if (rendered === null || typeof rendered !== 'object') { + return rendered; + } + + if (isArray(rendered)) { + const resolved: Array = []; + for (let i = 0; i < rendered.length; i++) { + resolved[i] = resolveModel(request, task, rendered, '' + i, rendered[i]); + } + return resolved; + } + + // Use `{}` for fast properties; `__proto__` is handled below because simple + // assignment would hit Object.prototype's setter instead of creating a key. + const resolved: {[key: string]: ReactJSONValue} = {} as any; + for (const key in rendered) { + if (hasOwnProperty.call(rendered, key)) { + const resolvedValue = resolveModel( + request, + task, + rendered, + key, + rendered[key], + ); + if (key === __PROTO__) { + // Match JSON's ordinary data-property semantics for this legacy key. + Object.defineProperty(resolved, key, { + value: resolvedValue, + enumerable: true, + writable: true, + configurable: true, + }); + } else { + resolved[key] = resolvedValue; + } + } + } + return resolved; +} + function serializeByValueID(id: number): string { return '$' + id.toString(16); } @@ -3618,7 +3679,7 @@ function renderModelDestructive( // TODO: Pop this. Since we currently don't have a point where we can pop the stack // this debug information will be used for errors inside sibling properties that // are not elements. Leading to the wrong attribution on the server. We could fix - // that if we switch to a proper stack instead of JSON.stringify's trampoline. + // that if we switch to a proper stack instead of resolveModel's recursive walk. // Attribution on the client is still correct since it has a pop. } @@ -5816,8 +5877,11 @@ function emitChunk( return; } // For anything else we need to try to serialize it using JSON. + // We resolve the model tree first in pure JS to avoid the C++->JS boundary + // overhead of JSON.stringify's replacer callback. + const resolvedModel = resolveModel(request, task, {'': value}, '', value); // $FlowFixMe[incompatible-type] stringify can return null for undefined but we never do - const json: string = stringify(value, task.toJSON); + const json: string = stringify(resolvedModel); emitModelChunk(request, task.id, json); } @@ -5863,7 +5927,7 @@ function retryTask(request: Request, task: Task): void { try { // Track the root so we know that we have to emit this object even though it // already has an ID. This is needed because we might see this object twice - // in the same toJSON if it is cyclic. + // in the same resolveModel walk if it is cyclic. modelRoot = task.model; if (__DEV__) { @@ -5922,8 +5986,8 @@ function retryTask(request: Request, task: Task): void { // This is simulating what the JSON loop would do if this was part of it. emitChunk(request, task, resolvedModel); } else { - // If the value is a string, it means it's a terminal value and we already escaped it - // We don't need to escape it again so it's not passed the toJSON replacer. + // If the value is a string, it means it's a terminal value and we already escaped it. + // We don't need to escape it again so it's not passed through resolveModel. // $FlowFixMe[incompatible-type] stringify can return null for undefined but we never do const json: string = stringify(resolvedModel); emitModelChunk(request, task.id, json);