Skip to content

Commit

Permalink
fix: JSON.stringify with replacer returning new object (#874)
Browse files Browse the repository at this point in the history 
* Fix nested object replacer

* clippy
  • Loading branch information
@richarddavison
richarddavison authored Mar 7, 2025
1 parent e092d7b commit 063e059
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 92 deletions.
104 changes: 53 additions & 51 deletions libs/llrt_json/benches/json.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,66 +2,68 @@
// SPDX-License-Identifier: Apache-2.0

#![allow(dead_code)]
use criterion::{black_box, criterion_group, criterion_main, Criterion};
use criterion::{black_box, criterion_group, criterion_main, BenchmarkId, Criterion};
use llrt_json::{parse::json_parse, stringify::json_stringify};
use rquickjs::{Context, Runtime};

static JSON: &str = r#"{"organization":{"name":"TechCorp","founding_year":2000,"departments":[{"name":"Engineering","head":{"name":"Alice Smith","title":"VP of Engineering","contact":{"email":"alice.smith@techcorp.com","phone":"+1 (555) 123-4567"}},"employees":[{"id":101,"name":"Bob Johnson","position":"Software Engineer","contact":{"email":"bob.johnson@techcorp.com","phone":"+1 (555) 234-5678"},"projects":[{"project_id":"P001","name":"Project A","status":"In Progress","description":"Developing a revolutionary software solution for clients.","start_date":"2023-01-15","end_date":null,"team":[{"id":201,"name":"Sara Davis","role":"UI/UX Designer"},{"id":202,"name":"Charlie Brown","role":"Quality Assurance Engineer"}]},{"project_id":"P002","name":"Project B","status":"Completed","description":"Upgrading existing systems to enhance performance.","start_date":"2022-05-01","end_date":"2022-11-30","team":[{"id":203,"name":"Emily White","role":"Systems Architect"},{"id":204,"name":"James Green","role":"Database Administrator"}]}]},{"id":102,"name":"Carol Williams","position":"Senior Software Engineer","contact":{"email":"carol.williams@techcorp.com","phone":"+1 (555) 345-6789"},"projects":[{"project_id":"P001","name":"Project A","status":"In Progress","description":"Working on the backend development of Project A.","start_date":"2023-01-15","end_date":null,"team":[{"id":205,"name":"Alex Turner","role":"DevOps Engineer"},{"id":206,"name":"Mia Garcia","role":"Software Developer"}]},{"project_id":"P003","name":"Project C","status":"Planning","description":"Researching and planning for a future project.","start_date":null,"end_date":null,"team":[]}]}]},{"name":"Marketing","head":{"name":"David Brown","title":"VP of Marketing","contact":{"email":"david.brown@techcorp.com","phone":"+1 (555) 456-7890"}},"employees":[{"id":201,"name":"Eva Miller","position":"Marketing Specialist","contact":{"email":"eva.miller@techcorp.com","phone":"+1 (555) 567-8901"},"campaigns":[{"campaign_id":"C001","name":"Product Launch","status":"Upcoming","description":"Planning for the launch of a new product line.","start_date":"2023-03-01","end_date":null,"team":[{"id":301,"name":"Oliver Martinez","role":"Graphic Designer"},{"id":302,"name":"Sophie Johnson","role":"Content Writer"}]},{"campaign_id":"C002","name":"Brand Awareness","status":"Ongoing","description":"Executing strategies to increase brand visibility.","start_date":"2022-11-15","end_date":"2023-01-31","team":[{"id":303,"name":"Liam Taylor","role":"Social Media Manager"},{"id":304,"name":"Ava Clark","role":"Marketing Analyst"}]}]}]}]}}"#;

static JSON_MIN: &str = r#"{"glossary":{"title":"example glossary","GlossDiv":{"title":"S","GlossList":{"GlossEntry":{"ID":"SGML","SortAs":"SGML","GlossTerm":"Standard Generalized Markup Language","Acronym":"SGML","Abbrev":"ISO 8879:1986","GlossDef":{"para":"A meta-markup language, used to create markup languages such as DocBook.","GlossSeeAlso":["GML","XML"]},"GlossSee":"markup"}}}}}"#;

// fn generate_json(child_json: &str, size: usize) -> String {
// let mut json = String::with_capacity(child_json.len() * size);
// json.push('{');
// for i in 0..size {
// json.push_str("\"obj");
// json.push_str(&i.to_string());
// json.push_str("\":");
// json.push_str(child_json);
// json.push(',');
// }
// json.push_str("\"array\":[");
// for i in 0..size {
// json.push_str(child_json);
// if i < size - 1 {
// json.push(',');
// }
// }

// json.push_str("]}");
// json
// }
fn generate_json(child_json: &str, size: usize) -> String {
let mut json = String::with_capacity(child_json.len() * size);
json.push('{');
for i in 0..size {
json.push_str("\"obj");
json.push_str(&i.to_string());
json.push_str("\":");
json.push_str(child_json);
json.push(',');
}
json.push_str("\"array\":[");
for i in 0..size {
json.push_str(child_json);
if i < size - 1 {
json.push(',');
}
}

json.push_str("]}");
json
}

pub fn criterion_benchmark(c: &mut Criterion) {
// let mut group = c.benchmark_group("Parsing");

// let json = JSON.to_owned();
// for (id, json) in [
// json.clone(),
// generate_json(&json, 1),
// generate_json(&json, 10),
// generate_json(&json, 100),
// ]
// .into_iter()
// .enumerate()
// {
// let runtime = Runtime::new().unwrap();
// runtime.set_max_stack_size(512 * 1024);

// let ctx = Context::full(&runtime).unwrap();
// group.bench_with_input(BenchmarkId::new("Custom", id), &json, |b, json| {
// ctx.with(|ctx| {
// b.iter(|| json_parse(&ctx, json.clone()));
// });
// });
// group.bench_with_input(BenchmarkId::new("Built-in", id), &json, |b, json| {
// ctx.with(|ctx| {
// b.iter(|| ctx.json_parse(json.clone()));
// });
// });
// }

// group.finish();
let mut group = c.benchmark_group("Parsing");

let json = JSON.to_owned();
for (id, json) in [
json.clone(),
generate_json(&json, 1),
generate_json(&json, 10),
generate_json(&json, 100),
generate_json(&json, 1000),
generate_json(&json, 10000),
]
.into_iter()
.enumerate()
{
let runtime = Runtime::new().unwrap();
runtime.set_max_stack_size(512 * 1024);

let ctx = Context::full(&runtime).unwrap();
group.bench_with_input(BenchmarkId::new("Custom", id), &json, |b, json| {
ctx.with(|ctx| {
b.iter(|| json_parse(&ctx, json.clone()));
});
});
group.bench_with_input(BenchmarkId::new("Built-in", id), &json, |b, json| {
ctx.with(|ctx| {
b.iter(|| ctx.json_parse(json.clone()));
});
});
}

group.finish();

c.bench_function("json parse", |b| {
let runtime = Runtime::new().unwrap();
Expand Down
78 changes: 38 additions & 40 deletions libs/llrt_json/src/stringify.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ pub fn json_stringify_replacer_space<'js>(

context.depth += 1;
context.indentation = indentation;
iterate(&mut context)?;
iterate(&mut context, None)?;
Ok(Some(result))
}

Expand Down Expand Up @@ -157,10 +157,10 @@ fn run_to_json<'js>(
}

#[derive(PartialEq)]
enum PrimitiveStatus {
enum PrimitiveStatus<'js> {
Written,
Ignored,
Iterate,
Iterate(Option<Value<'js>>),
}

#[inline(always)]
Expand All @@ -169,62 +169,57 @@ fn run_replacer<'js>(
context: &mut StringifyContext<'_, 'js>,
replacer_fn: &Function<'js>,
add_comma: bool,
) -> Result<PrimitiveStatus> {
let parent = context.parent;
let ctx = context.ctx;
let value = context.value;
) -> Result<PrimitiveStatus<'js>> {
let key = context.key;
let index = context.index;
let parent = if let Some(parent) = parent {
let value = context.value;
let parent = if let Some(parent) = context.parent {
parent.clone()
} else {
let parent = Object::new(ctx.clone())?;
let parent = Object::new(context.ctx.clone())?;
parent.set("", value.clone())?;
parent
};
let new_value = replacer_fn.call((
let new_value: Value = replacer_fn.call((
This(parent),
get_key_or_index(context.itoa_buffer, key, index),
value,
))?;
write_primitive(
&mut StringifyContext {
ctx,
result: context.result,
value: &new_value,
replacer_fn: None,
key,
index: None,
indentation: context.indentation,
parent: None,
include_keys_replacer: None,
depth: context.depth,
ancestors: context.ancestors,
itoa_buffer: context.itoa_buffer,
ryu_buffer: context.ryu_buffer,
},
add_comma,
)

write_primitive2(context, add_comma, Some(new_value))
}

fn write_primitive(context: &mut StringifyContext, add_comma: bool) -> Result<PrimitiveStatus> {
fn write_primitive<'js>(
context: &mut StringifyContext<'_, 'js>,
add_comma: bool,
) -> Result<PrimitiveStatus<'js>> {
if let Some(replacer_fn) = context.replacer_fn {
return run_replacer(context, replacer_fn, add_comma);
}

let include_keys_replacer = context.include_keys_replacer;
let value = context.value;
write_primitive2(context, add_comma, None)
}

fn write_primitive2<'js>(
context: &mut StringifyContext<'_, 'js>,
add_comma: bool,
new_value: Option<Value<'js>>,
) -> Result<PrimitiveStatus<'js>> {
let key = context.key;
let index = context.index;
let include_keys_replacer = context.include_keys_replacer;
let indentation = context.indentation;
let depth = context.depth;

let value = new_value.as_ref().unwrap_or(context.value);

let type_of = value.type_of();

if matches!(
type_of,
Type::Symbol | Type::Undefined | Type::Function | Type::Constructor
) && context.index.is_none()
if context.index.is_none()
&& matches!(
type_of,
Type::Symbol | Type::Undefined | Type::Function | Type::Constructor
)
{
return Ok(PrimitiveStatus::Ignored);
}
Expand Down Expand Up @@ -289,7 +284,7 @@ fn write_primitive(context: &mut StringifyContext, add_comma: bool) -> Result<Pr
context.result,
&unsafe { value.as_string().unwrap_unchecked() }.to_string()?,
),
_ => return Ok(PrimitiveStatus::Iterate),
_ => return Ok(PrimitiveStatus::Iterate(new_value)),
}
Ok(PrimitiveStatus::Written)
}
Expand Down Expand Up @@ -374,9 +369,9 @@ fn append_value(context: &mut StringifyContext<'_, '_>, add_comma: bool) -> Resu
match write_primitive(context, add_comma)? {
PrimitiveStatus::Written => Ok(true),
PrimitiveStatus::Ignored => Ok(false),
PrimitiveStatus::Iterate => {
PrimitiveStatus::Iterate(new_value) => {
context.depth += 1;
iterate(context)?;
iterate(context, new_value)?;
Ok(true)
},
}
Expand Down Expand Up @@ -419,10 +414,13 @@ fn get_key_or_index<'a>(
key.unwrap_or_else(|| itoa_buffer.format(index.unwrap_or_default()))
}

fn iterate(context: &mut StringifyContext<'_, '_>) -> Result<()> {
fn iterate<'js>(
context: &mut StringifyContext<'_, 'js>,
new_value: Option<Value<'js>>,
) -> Result<()> {
let mut add_comma;
let mut value_written;
let elem = context.value;
let elem = new_value.as_ref().unwrap_or(context.value);
let depth = context.depth;
let ctx = context.ctx;
let indentation = context.indentation;
Expand Down
22 changes: 21 additions & 1 deletion tests/unit/json.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ describe("JSON Stringified", () => {
});

it("should stringify and remove objects that are not valid json", () => {
const now = new Date()
const now = new Date();
const dateString = now.toJSON();
const data = {
a: "123",
Expand Down Expand Up @@ -327,4 +327,24 @@ describe("JSON Stringified", () => {
})
).toThrow(/Do not know how to serialize a BigInt/);
});

it("should allow replacer that returns new non-primitive objects", () => {
const json = JSON.stringify({ key: "value" });

const obj = {
simple: "text",
nested: json,
};

const replacer = (key: any, value: any) => {
try {
return typeof value === "string" ? JSON.parse(value) : value;
} catch {
return value;
}
};

const result = JSON.stringify(obj, replacer);
expect(result).toEqual('{"simple":"text","nested":{"key":"value"}}');
});
});

0 comments on commit 063e059

Please sign in to comment.