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

Docs: Show errors when stories throw in docs #21212

Merged
merged 21 commits into from
Feb 27, 2023
Merged

Docs: Show errors when stories throw in docs #21212

merged 21 commits into from
Feb 27, 2023

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Feb 23, 2023

Closes N/A

What I did

Made it so errors show up in docs mode:

image

image

Not perfect but a lot better than the nothing it showed before.

How to test

Create a story with an error, check it in docs.

Checklist

  • Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
  • Make sure to add/update documentation regarding your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Maintainers

  • If this PR should be tested against many or all sandboxes,
    make sure to add the ci:merged or ci:daily GH label to it.
  • Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

this.channel,
this.storyStore,
this.renderToCanvas,
this.inlineStoryCallbacks(story.id),
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously we passed error callbacks that basically did nothing.

story,
element,
{
showMain: () => {},
Copy link
Member Author

Choose a reason for hiding this comment

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

We could use this to show a spinner or something until the story has rendered but I'm not sure there's much call for that.

Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

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

Great work! A few minor comments, but none blocking.

code/e2e-tests/addon-docs.spec.ts Outdated Show resolved Hide resolved
code/ui/blocks/src/examples/Button.stories.tsx Outdated Show resolved Hide resolved
Co-authored-by: Jeppe Reinhold <jeppe@chromatic.com>
@tmeasday
Copy link
Member Author

From @ndelangen :

Here’s a component we might be able to use/repurpose for displaying the error more neatly:

const ErrorFormatter: FC<{ error: Error }> = ({ error }) => {
if (!error) {
return <Fragment>This error has no stack or message</Fragment>;
}
if (!error.stack) {
return <Fragment>{error.message || 'This error has no stack or message'}</Fragment>;
}
const input = error.stack.toString();
const match = input.match(firstLineRegex);
if (!match) {
return <Fragment>{input}</Fragment>;
}
const [, type, name] = match;
const rawLines = input.split(/\n/).slice(1);
const [, ...lines] = rawLines
.map((line) => {
const r = line.match(linesRegex);
return r ? { name: r[1], location: r[2].replace(document.location.origin, '') } : null;
})
.filter(Boolean);
return (
<Fragment>
<span>{type}</span>: <ErrorName>{name}</ErrorName>
<br />
{lines.map((l, i) =>
l.name ? (
// eslint-disable-next-line react/no-array-index-key
<Fragment key={i}>
{' '}at <ErrorImportant>{l.name}</ErrorImportant> (
<ErrorDetail>{l.location}</ErrorDetail>)
<br />
</Fragment>
) : (
// eslint-disable-next-line react/no-array-index-key
<Fragment key={i}>
{' '}at <ErrorDetail>{l.location}</ErrorDetail>
<br />
</Fragment>
)
)}
</Fragment>
);
};

@tmeasday
Copy link
Member Author

I updated to use @ndelangen's component. It looks better in Chrome, arguably worse in other browsers. I'll just merge and we can iterate -- @MichaelArestad when you have a min you can just work directly on the Story component: http://localhost:6010/?path=/story/storybook-blocks-components-story--inline-error or even ErrorFormatter: http://localhost:6010/?path=/story/storybook-components-errorformatter--chrome

@ndelangen ndelangen self-assigned this Feb 24, 2023
@ndelangen
Copy link
Member

I found out why the e2e test were failing (and why only for safari and firefox!)

The stacktrace in safari and firefox do not contain the error.message.

I improved the ErrorFormatter component to handle these cases better.

@MichaelArestad
Copy link
Contributor

I will take a look at this on Monday. I am also doing a pass at styling docs in general and I could use a bit of help.

@tmeasday
Copy link
Member Author

Thanks for sorting this @ndelangen. I guess we can wait to merge this then if @MichaelArestad is look at it so soon.

@ndelangen
Copy link
Member

@shilman I'm going to merge this when it's green

@ndelangen ndelangen merged commit c8acbf3 into next Feb 27, 2023
@ndelangen ndelangen deleted the show-docs-errors branch February 27, 2023 12:04
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