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

componentDidCatch: document & decompose info parameter #10461

Closed
brandonbloom opened this issue Aug 15, 2017 · 9 comments
Closed

componentDidCatch: document & decompose info parameter #10461

brandonbloom opened this issue Aug 15, 2017 · 9 comments
Assignees

Comments

@brandonbloom
Copy link
Contributor

Some quick googling didn't show any explanation of the info parameter for componentDidCatch(error, info). The only key in that object that I've seen so far is componentStack, which is useful, but is already composed. If I want to do some custom rendering of that information, I have to parse it.

I'd love to see info.componentStack become a more general info.text string and then have componentStack be broken down to an array of frames. Something like [{component, createdBy}].

@bvaughn
Copy link
Contributor

bvaughn commented Aug 15, 2017

This is a new feature, to be introduced in 16 final. We'll add documentation for it soon but probably not until we release an RC.

I don't expect the 2nd parameter, (the one containing componentStack), to change in the short-term for what it's worth. We went with a named parameter for that because it gives us the option to add more information in a backwards-compatible way.

@brandonbloom
Copy link
Contributor Author

brandonbloom commented Aug 15, 2017

I was not proposing to change it from an object with keys, I was proposing to add/rename keys so as to include the trace as decomposable data.

// example call from framework to my component
that.componentDidCatch(err, {
  text: 'Something Bad Happened\nat Foo (Bar) \n...',
  componentStack: [
    {component: 'Foo', createdBy: 'Bar'},
    ...
  ]
})

@bvaughn
Copy link
Contributor

bvaughn commented Aug 15, 2017

Understood! I was just offering an explanation for why we used a named-parameter for a single param 😄 and why there wasn't any docs entry for this yet (since it's only available in beta).

ReactFiberComponentTreeHook has all of the info you mention so it wouldn't be hard to pass it along. (And I think this issue is great to keep around for one of us to follow-up on.)

@bvaughn
Copy link
Contributor

bvaughn commented Aug 17, 2017

@bvaughn
Copy link
Contributor

bvaughn commented Aug 17, 2017

Having given this more thought, I'm thinking the following change might be nice:

type Source = {
  fileName: string,
  lineNumber: number,
};

type StackFrame = {
  name: string | null,
  ownerName: string | null,
  source: Source | null,
};

// Passed to the function injected via ReactFiberErrorLogger.injection.injectDialog:
// injectedFunction(capturedError: CapturedError);
type CapturedError = {
  componentName: ?string,
  componentStack: string,
  componentStackFrames: Array<StackFrame>,
  error: mixed,
  errorBoundary: ?Object,
  errorBoundaryFound: boolean,
  errorBoundaryName: string | null,
  willRetry: boolean,
};

// Passed to the componentDidCatch lifecycle hook:
// componentDidCatch(error: Error, info: ErrorInfo);
type ErrorInfo = {
  componentStack: string,
  componentStackFrames: Array<StackFrame>,
};

Since this proposal is backwards compatible with the beta method signature, I'm going to take a stab at it.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

Closing in favor of #10461 which has more activity.

@gaearon gaearon closed this as completed Oct 4, 2017
@bvaughn
Copy link
Contributor

bvaughn commented Oct 4, 2017

This issue is #10461 😄

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

Oh man.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

#10922

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 a pull request may close this issue.

3 participants