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

Reference docs for experimental captureOwnerStack #7427

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

eps1lon
Copy link
Collaborator

@eps1lon eps1lon commented Jan 13, 2025

Based on #7430

Preview:
captureOwnerStack
createRoot error handlers
hydrateRoot error handlers

The owner stack vs parent section probably deserves its own page since owner stacks also appear in devtools and console.createTask.

Should I just move this into a "debugging" section inside "learn"?

Copy link

vercel bot commented Jan 13, 2025

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

Name Status Preview Comments Updated (UTC)
19-react-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 4:38pm
react-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 4:38pm

Copy link

github-actions bot commented Jan 13, 2025

Size changes

📦 Next.js Bundle Analysis for react-dev

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page Size (compressed)
global 110.38 KB (🟡 +2 B)
Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Five Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page Size (compressed) First Load
/404 125.09 KB (🟡 +18 B) 235.47 KB
/500 125.09 KB (🟡 +18 B) 235.48 KB
/[[...markdownPath]] 126.97 KB (🟡 +34 B) 237.35 KB
/errors 125.37 KB (🟡 +18 B) 235.75 KB
/errors/[errorCode] 125.34 KB (🟡 +18 B) 235.72 KB
Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 10% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

I think the naming we should go with is:

  • Component Stack: what you're calling "parent component stack" here
  • Owner Stack: what you're calling "owner component stack" here

* **optional** `onCaughtError`: Callback called when React catches an error in an Error Boundary. Called with the `error` caught by the Error Boundary, and an `errorInfo` object containing the `componentStack`.
* **optional** `onUncaughtError`: Callback called when an error is thrown and not caught by an Error Boundary. Called with the `error` that was thrown, and an `errorInfo` object containing the `componentStack`.
* **optional** `onRecoverableError`: Callback called when React automatically recovers from errors. Called with an `error` React throws, and an `errorInfo` object containing the `componentStack`. Some recoverable errors may include the original error cause as `error.cause`.
* **optional** `onCaughtError`: Callback called when React catches an error in an Error Boundary. Called with the `error` caught by the Error Boundary, and an `errorInfo` object containing the parent Component stack in `componentStack`.
Copy link
Member

Choose a reason for hiding this comment

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

What is a "parent Component Stack" isn't that just the Component Stack? Adding "parent" makes it sound like it doesn't include the component that errored.

This page should also add a usage item for calling captureOwnerStack to get the owner stack.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I feel like expanding the existing usages is sufficient. Is that what you meant? Adding a whole new section feels like overkill.

src/content/reference/react/captureOwnerStack.md Outdated Show resolved Hide resolved
@@ -369,7 +369,7 @@ root.render(<App />);
The <CodeStep step={1}>onUncaughtError</CodeStep> option is a function called with two arguments:

1. The <CodeStep step={2}>error</CodeStep> that was thrown.
2. An <CodeStep step={3}>errorInfo</CodeStep> object that contains the <CodeStep step={4}>componentStack</CodeStep> of the error.
2. An <CodeStep step={3}>errorInfo</CodeStep> object that contains the parent Component stack in <CodeStep step={4}>componentStack</CodeStep> of the error.
Copy link
Member

Choose a reason for hiding this comment

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

The capitalization of only "Component" seems weird, seems like it should either be Component Stack or component stack.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

renamed
"parent Component stack" -> "Component Stack"
"owner Component stack" -> "Owner Stack"

If no owner stack is available, it returns an empty string.
Outside of development builds, `null` is returned.

## Owner Component stacks vs parent Component stacks {/*owner-component-stacks-vs-parent-component-stacks*/}
Copy link
Member

Choose a reason for hiding this comment

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

Let's wrap this in a <DeepDive />

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

src/content/reference/react/captureOwnerStack.md Outdated Show resolved Hide resolved

Neither `Navigation` nor `legend` are in the stack at all since it's only a sibling to a node containing `<SubComponent />`.

### When to use which {/*when-to-use-which*/}
Copy link
Member

Choose a reason for hiding this comment

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

This explanation seems too technical to really understand when to use each. This is more "what they represent" and not "when to use". It's not really an either/or either. Owner stacks can be added in addition to component stacks when you're in DEV and want to add additional debugging info.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not really an either/or either.

I agree. This is not what I wanted to convey with this section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed the section. It didn't really say anything new we already explained earlier.

onCaughtError: (error, errorInfo) => {
if (process.env.NODE_ENV !== 'production') {
const ownerStack = captureOwnerStack();
error.stack += ownerStack;
Copy link
Member

Choose a reason for hiding this comment

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

Do we really recommend appending it to the existing stack? Won't that add it to the end?

IMO this should be like:

if (__DEV__) { 
  showDialog(error, ownerStack, componentStack);
} else {
  logProdError(error, componentStack);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We do that in Next.js and it's conceptually how you arrive at the error if every JSX callsite would just be a function call.


<Intro>

`captureOwnerStack` reads the current **owner** Component stack and returns it as a string if available.
Copy link
Member

Choose a reason for hiding this comment

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

We should make it more clear that this is dev-only. I would mention it here, in a Caveat, and a Troubleshooting "My owner stacks are empty in production" section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Did all that. Also explained this in the createRoot and hydrateRoot usage examples. Though possible I missed some spots.

@sophiebits
Copy link
Member

Aren't we getting rid of parent stacks entirely eventually? I thought the plan was for owner stacks to subsume them. In which case I might still discuss which stack frames you'll see but not emphasize the difference so much.

@sebmarkbage
Copy link
Contributor

sebmarkbage commented Jan 13, 2025

Aren't we getting rid of parent stacks entirely eventually? I thought the plan was for owner stacks to subsume them.

Not for production logging. They're the ones passed to the deriveStateFromError/componentDidCatch and onError on the server. They only go away in the standard DEV console.error reporting.

@rickhanlonii
Copy link
Member

There should also be runable sandboxes for the Usage examples

@gaearon
Copy link
Member

gaearon commented Jan 14, 2025

There should also be runable sandboxes for the Usage examples

To expand on that — it’s not just for it to be nice but because it’s a proof the API actually works end to end and does what the docs page says it does. We’ve had cases where that wasn’t the case and the Usage examples helped uncover the misunderstandings.

eps1lon and others added 9 commits January 14, 2025 16:49
Forks on CodeSandbox will ignore `/index.html` and create their own `public/index.html` which leads to broken sandboxes in CodeSandbox.

We now throw if `/index.html` is used and only use `public/index.html` which works both in Sandpack and CodeSandbox.

This also means we're consistent. Some sandboxes were already using the functioning `public/index.html`.
Co-authored-by: Ricky <rickhanlonii@fb.com>
parent Component stack -> Component Stack
owner Component stack -> Owner Stack
Was just rephrasing what we already had and gave wrong impression
i.e. everywhere I found mention of `"componentStack"`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants