Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ecmascript): Implement JSON.stringify #527

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

eliassjogreen
Copy link
Member

I probably need to do some scoping/binding magic with some of my values still.

Copy link
Collaborator

@aapoalas aapoalas left a comment

Choose a reason for hiding this comment

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

Some comments at least from a quick glance.

Making this GC-safe is going to be a pain with the tools that we have now. I have to create a way to scope collections.

}
}

impl From<PrimitiveObjectData> for Value {
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion: IntoPrimitive as well? And perhaps create IntoValue based on that?

@@ -317,6 +528,424 @@ fn internalize_json_property<'a>(
)
}

struct JSONSerializationRecord {
replacer_function: Option<Function<'static>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

note: These should rather be scoped values: Function<'static> together with a GcScope basically just means "first sign of GC, and this is all invalid" :)

fn serialize_json_property(
agent: &mut Agent,
state: &mut JSONSerializationRecord,
key: String<'static>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

note: You want to take a PropertyKey here and not call ToString on numbers too early. You'll want to instead only serialize those into strings when you're writing them into the JSON buffer, and then wrap them with " to get "automatic" formatting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants