lookbook-hq / lookbook

A UI development environment for Ruby on Rails apps ✨
https://lookbook.build
MIT License
853 stars 90 forks source link

Syntax error reporting in Lookbook #593

Open Jackson200 opened 4 months ago

Jackson200 commented 4 months ago

Problem

Hello all. When the preview file has a ruby error is it declared as missing instead of reporting the error

To Reproduce

Steps to reproduce the behavior:

  1. Work on a preview file

  2. Change something that breaks the code. The preview now cannot be found: Screenshot 2024-02-13 at 10 45 43 AM

  3. Go to the rails preview to get a clue of the error Screenshot 2024-02-13 at 10 46 17 AM

Desired behaviour

It would be great if we could see this error on Lookbook.

Version numbers

Additional context

Thanks for looking! And the continued work :) We love Lookbook! 🚀

allmarkedup commented 2 months ago

Hey @Jackson200, so sorry for being so slow to reply and thanks for opening this as an issue.

I totally agree that the behaviour you suggest is desirable, however it's actually a little tricky because of the way Lookbook finds and parses preview files.

Because (unlike with vanilla ViewComponent previews) Lookbook pre-parses the Preview files to extract info from the comment annotations, any syntax errors actually mean that the parser often cannot correctly extract the data from that preview file. And without that data Lookbook can't display the preview or scenarios in the navigation, so it is never included in the list of 'known' previews, which is why you see the 'preview not found' 404 page instead.

This is definitely not ideal! There are a number of ways this could be handled differently - certainly at a minimum parser errors could be surfaced in the UI somewhere, although in many cases it may still not be possible to avoid the preview being removed from the nav (and the associated 404 response).

I'm not sure if that explaination makes much sense but this is definitely a feature request I'll keep open as I'm sure this is a pretty common pain point for people using Lookbook.

Thanks for looking! And the continued work :) We love Lookbook! 🚀

Thank you - always good to know, really pleased you find it useful 🙂

Jackson200 commented 2 months ago

Thanks for the explanation @allmarkedup . That makes total sense.

Parser errors in the UI would really be great, and I think even breaking out of Lookbook completely would be acceptable to me, depending on the level of info shown in the parsing error.

Thanks for looking!