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

Bundle error dialog enhancements #625

Merged
merged 2 commits into from
Oct 15, 2024
Merged

Bundle error dialog enhancements #625

merged 2 commits into from
Oct 15, 2024

Conversation

kmagiera
Copy link
Member

@kmagiera kmagiera commented Oct 15, 2024

This PR updates the error dialog we show when detecting bundle error.

Previously, we'd direct the users to app logs from that dialog. Those logs apparently never display any valuable information about bundling as they come from the app process, while bundle errors are reported by metro.

In addition, the reload button from bundle error would request JS reload, which isn't sufficient in the case when the app boots up with a bundle error.

We are therefore making the following changes:

  1. We're updating the logs button from bundle error dialog to open IDE logs where metro errors are getting printed: image
  2. We're changing the reload button to call restart – restart implementation is smart enough and will attempt to reload JS when the app process is running, otherwise it will try to restart the app process, which is also good in case when we're recovering from initial bundle error
  3. We're also updating reload implementation to remove "TODO" added there. The TODO was about handling unsuccessful reloads. Those could happen when the user forces "reloadJS" but the app process isn't running. In such a case we shouldn't attempt another reload method as that's not what user requested. Instead we will show an error popup with message that the reload attempt wasn't successful.

How Has This Been Tested:

  1. Delete 51-dev-client folder and checkout a fresh copy.
  2. Run the IDE on that new project and expect bundle error.
  3. See that logs button opens IDE logs which shows error about missing shared files
  4. Click "reload JS" button from the refresh dropdown to see new error dialog
  5. Use copy-shared script to copy them and click reload
  6. The app will restart and load properly
  7. Add import to something that doesn't exists
  8. See the bundle error again, this one on top of a running app
  9. Fix error and click reload – it should only reload JS and recover immediately

Copy link

vercel bot commented Oct 15, 2024

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

Name Status Preview Comments Updated (UTC)
radon-ide ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 15, 2024 1:17pm

Copy link
Member

@jakub-gonet jakub-gonet left a comment

Choose a reason for hiding this comment

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

Would be nice to always add tags with name of the subsystem in logs in the future so it can be filtered more easily.

@kmagiera kmagiera merged commit f5baec5 into main Oct 15, 2024
3 checks passed
@kmagiera kmagiera deleted the kmagiera/bundle-errors branch October 15, 2024 19:41
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 this pull request may close these issues.

2 participants