Skip to content
Merged
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
75 changes: 18 additions & 57 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

116 changes: 109 additions & 7 deletions specta-serde/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1199,6 +1199,12 @@ fn rewrite_field_for_phase(
{
if let PhaseRewrite::Serialize = mode {
field.optional = true;

if attrs.skip_serializing_if.as_deref() == Some("Option::is_none")
&& let Some(DataType::Nullable(inner)) = field.ty.take()
{
field.ty = Some(*inner);
}
}
// The attribute is meaningless on phase-split fields: the _Serialize
// variant already has `optional = true`, and the _Deserialize variant
Expand Down Expand Up @@ -1423,20 +1429,48 @@ fn enum_repr_already_rewritten(e: &Enum) -> bool {
}

fn variant_repr_already_rewritten(name: &str, variant: &Variant) -> bool {
// `transform_internal_variant` produces a tag of type `Primitive::str` when
// `widen_tag` is set (the `#[serde(other)]` variant in deserialize phase),
// and produces a tuple-variant body of `Intersection<{tag-struct}, payload>`
// for non-empty unnamed variants. Without recognizing both shapes, the
// second-pass guard on internally-tagged enums containing such variants
// fails. The whole enum then gets re-transformed -- this time as
// `EnumRepr::External`, because the first pass cleared `e.attributes` --
// producing wrong externally-tagged TS bindings for what serde actually
// emits as internally-tagged JSON.
fn is_rewritten_tag_ty(ty: &DataType) -> bool {
is_generated_string_literal_datatype(ty)
|| matches!(ty, DataType::Primitive(Primitive::str))
}

fn is_rewritten_internal_intersection(ty: &DataType) -> bool {
let DataType::Intersection(parts) = ty else {
return false;
};
let Some(DataType::Struct(s)) = parts.first() else {
return false;
};
let Fields::Named(named) = &s.fields else {
return false;
};
named
.fields
.iter()
.any(|(_, field)| field.ty.as_ref().is_some_and(is_rewritten_tag_ty))
}

match &variant.fields {
Fields::Unit => false,
Fields::Unnamed(fields) if name.is_empty() => unnamed_live_field_count(fields) == 1,
Fields::Unnamed(fields) if fields.fields.len() == 1 => fields
.fields
.first()
.and_then(|field| field.ty.as_ref())
.is_some_and(is_generated_string_literal_datatype),
.is_some_and(|ty| {
is_generated_string_literal_datatype(ty) || is_rewritten_internal_intersection(ty)
}),
Fields::Named(fields) => fields.fields.iter().any(|(field_name, field)| {
field_name == name
|| field
.ty
.as_ref()
.is_some_and(is_generated_string_literal_datatype)
field_name == name || field.ty.as_ref().is_some_and(is_rewritten_tag_ty)
}),
_ => false,
}
Expand Down Expand Up @@ -1870,6 +1904,40 @@ fn transform_internal_variant(
match &variant.fields {
Fields::Unit => {}
Fields::Named(named) => {
// If any named field is `#[serde(flatten)]`, serde merges its
// contents at the variant's top level alongside the tag. Mirror
// the unnamed-payload path: build an Intersection of `{tag}`,
// each flattened field's payload type, and (if any) a struct of
// the remaining non-flattened fields. Without this, the flatten
// attribute survives to the typescript exporter, which writes
// the field literally as `inner: T` instead of merging it.
let has_flattened = named
.fields
.iter()
.any(|(_, field)| field_is_flattened(field));
if has_flattened {
let mut payload_parts: Vec<DataType> = Vec::new();
let mut leftover: Vec<(Cow<'static, str>, Field)> = Vec::new();
for (name, field) in named.fields.iter().cloned() {
if field_is_flattened(&field) {
if let Some(ty) = field.ty {
payload_parts.push(ty);
}
} else {
leftover.push((name, field));
}
}
let mut intersection = Vec::with_capacity(payload_parts.len() + 2);
intersection.push(named_fields_datatype(fields));
if !leftover.is_empty() {
intersection.push(named_fields_datatype(leftover));
}
intersection.extend(payload_parts);
return Ok(clone_variant_with_unnamed_fields(
variant,
vec![Field::new(DataType::Intersection(intersection))],
));
}
fields.extend(named.fields.iter().cloned());
}
Fields::Unnamed(unnamed) => {
Expand Down Expand Up @@ -2621,6 +2689,29 @@ mod tests {
));
}

#[test]
fn skip_serializing_if_option_is_none_omits_null_in_serialize_phase() {
let mut types = specta::Types::default();
let dt = WithSkipIf::definition(&mut types);
let resolved = formatted_phases(types);

let serialize = select_phase_datatype(&dt, &resolved, Phase::Serialize);
let deserialize = select_phase_datatype(&dt, &resolved, Phase::Deserialize);

let serialize_field = named_field(&serialize, &resolved, "nickname");
let deserialize_field = named_field(&deserialize, &resolved, "nickname");

assert!(serialize_field.optional);
assert!(!matches!(
serialize_field.ty.as_ref(),
Some(DataType::Nullable(_))
));
assert!(matches!(
deserialize_field.ty.as_ref(),
Some(DataType::Nullable(_))
));
}

#[test]
fn phase_split_field_passes_unified_mode_validation() {
// Regression test for the interaction with downstream callers (e.g.
Expand Down Expand Up @@ -2670,6 +2761,17 @@ mod tests {
}

fn named_field_type<'a>(dt: &'a DataType, types: &'a Types, field_name: &str) -> &'a DataType {
named_field(dt, types, field_name)
.ty
.as_ref()
.expect("field should have a type")
}

fn named_field<'a>(
dt: &'a DataType,
types: &'a Types,
field_name: &str,
) -> &'a specta::datatype::Field {
let DataType::Reference(specta::datatype::Reference::Named(reference)) = dt else {
panic!("expected named reference");
};
Expand All @@ -2685,7 +2787,7 @@ mod tests {
fields
.fields
.iter()
.find_map(|(name, field)| (name == field_name).then_some(field.ty.as_ref()).flatten())
.find_map(|(name, field)| (name == field_name).then_some(field))
.expect("field should exist")
}

Expand Down
24 changes: 24 additions & 0 deletions tests/tests/serde_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ struct SkipSerializingIfOnly {
value: Option<String>,
}

#[derive(Type, Serialize, Deserialize)]
#[specta(collect = false)]
struct DefaultedSkipSerializingIfOnly {
#[serde(default, skip_serializing_if = "Option::is_none")]
value: Option<String>,
}

#[derive(Type, Serialize, Deserialize)]
#[specta(collect = false)]
enum TupleVariantSkipSerializingIfOnly {
Expand Down Expand Up @@ -412,6 +419,23 @@ fn skip_serializing_if_requires_phases() {
.expect("PhasesFormat should accept skip_serializing_if");
}

#[test]
fn option_is_none_omits_null_in_serialize_phase() {
let rendered = Typescript::default()
.export(
&Types::default()
.register::<SkipSerializingIfOnly>()
.register::<DefaultedSkipSerializingIfOnly>(),
specta_serde::PhasesFormat,
)
.expect("PhasesFormat should split skip_serializing_if phases");

insta::assert_snapshot!(
"serde-conversions-format-phases-option-is-none-omits-null",
rendered
);
}

#[test]
fn tuple_variant_skip_serializing_if_requires_phases_and_splits_owner() {
let err = Typescript::default()
Expand Down
9 changes: 7 additions & 2 deletions tests/tests/snapshots/test__jsdoc__export-many-serde.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: tests/tests/jsdoc.rs
assertion_line: 143
expression: output
---
/**
Expand Down Expand Up @@ -729,8 +730,12 @@ expression: output
* }} C
* @property {First} D
*
* @typedef {{ type: "Variant"; inner: First }} MyEnumTagged
* @property {{ type: "Variant"; inner: First }} Variant
* @typedef {{
* type: "Variant",
* } & First} MyEnumTagged
* @property {{
* type: "Variant",
* } & First} Variant
*
* @typedef {{ Variant: {
* inner: First,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
source: tests/tests/jsdoc.rs
assertion_line: 143
expression: output
---
/**
Expand Down Expand Up @@ -779,8 +780,12 @@ expression: output
* }} C
* @property {First} D
*
* @typedef {{ type: "Variant"; inner: First }} MyEnumTagged
* @property {{ type: "Variant"; inner: First }} Variant
* @typedef {{
* type: "Variant",
* } & First} MyEnumTagged
* @property {{
* type: "Variant",
* } & First} Variant
*
* @typedef {{ Variant: {
* inner: First,
Expand Down
Loading
Loading