Skip to content

Conversation

@sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Oct 9, 2024

We can't make a special getter to mark the boundary of deep serialization (which can be used for lazy loading in the future) when the parent object is a special object that we parse with getOutlinedModel. Such as Map/Set and JSX.

This marks the objects that are direct children of those as not possible to limit.

I don't love this solution since ideally it would maybe be more local to the serialization of a specific object.

It also means that very deep trees of only Map/Set never get cut off. Maybe we should instead override the get() and enumeration methods on these instead somehow.

It's important to have it be a getter though because that's the mechanism that lets us lazy-load more depth in the future.

@sebmarkbage sebmarkbage requested a review from eps1lon October 9, 2024 18:36
@vercel
Copy link

vercel bot commented Oct 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 9, 2024 6:37pm

@react-sizebot
Copy link

The size diff is too large to display in a single comment. The GitHub action for this pull request contains an artifact called 'sizebot-message.md' with the full message.

Generated by 🚫 dangerJS against 8b31d7d

@eps1lon
Copy link
Collaborator

eps1lon commented Oct 9, 2024

It also means that very deep trees of only Map/Set never get cut off.

Do we special case cycles? During debugging I set the limit to +inf and hit errors from JSON.stringify due to serialization of cyclic dependencies. That wasn't specific to Map/Set though if I remember correctly

@sebmarkbage
Copy link
Collaborator Author

We don't handle cyclic dependencies yet, no. But it would only be an issue if it's a cyclic Map directly in another Map. As soon as any other object is between it's fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants