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

Core: bail preview builder on failure #19849

Merged
merged 2 commits into from
Nov 17, 2022
Merged

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Nov 16, 2022

Issue: N/A

What I did

For some reason, even when Webpack fails (e.g. wrong main.js config), the preview may continue to print to stdout, which can affect output when we catch this error and process those errors (e.g. telemetry) gets overwritten by preview progress output. Therefore, we should bail the preview too.

An example to get into this situation is using a CRA project without @storybook/preset-create-react-app

The user experience is the following:

  • There's a misconfiguration which makes it so that Storybook partially fails. For instance, missing babel presets. This is not a fatal failure, though it should because presets require rerunning the server.
  • Storybook is not automatically opened, but can be accessed:

image

- The Storybook server says `WARN Force closed manager build` in the terminal - If crashReports were never answered, the telemetry prompts the user "would you like to send reports". However, immediately after that happens, Webpack continues with preview's progress, which in turn, ovewrites the stdout, therefore never showing the telemetry prompt - Because there is a prompt, the process is hanging, but the user doesn't even know why as they don't see the actual message. Only after pressing either Y, N or Ctrl+C, the process continues and the failures are shown. - This won't happen again in case users responded with Y or N, because that answer is saved in the cache.

How does it look like?

2022-11-16 11 57 04

So the fix now interrupts the preview process output, therefore showing the prompt correctly:

2022-11-16 11 58 01

And after the prompt is answered (and answer is cached), the other times the failure happens, users will get the errors directly:

image

How to test

  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

For some reason, even when Webpack fails e.g. wrong main.js config,
the preview may continue to print to stdout, which can affect output
when we catch this error and process those errors (e.g. telemetry)
gets overwritten by preview progress output. Therefore, we should bail the preview too.

An example to get into this situation is using a CRA project without @storybook/preset-create-react-app
@yannbf yannbf added the core label Nov 16, 2022
@yannbf yannbf requested review from shilman and ndelangen November 16, 2022 11:08
@yannbf yannbf added the bug label Nov 16, 2022
@ndelangen ndelangen self-assigned this Nov 17, 2022
@ndelangen ndelangen merged commit c8e52a0 into next Nov 17, 2022
@ndelangen ndelangen deleted the fix/bail-preview-on-failure branch November 17, 2022 17:11
@yannbf yannbf added the linear label Nov 21, 2022
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.

2 participants