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

[Flight] Don't limit objects that are children of special types #31160

Merged
merged 2 commits into from
Oct 10, 2024

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.

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