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

Syntax error reporting in Lookbook #593

Open
Jackson200 opened this issue Feb 13, 2024 · 2 comments
Open

Syntax error reporting in Lookbook #593

Jackson200 opened this issue Feb 13, 2024 · 2 comments
Labels
feature request A suggestion for a feature or enhancement

Comments

@Jackson200
Copy link

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

  • Lookbook: 2.2.0
  • ViewComponent: 3.7.0
  • Rails: 7.0.8
  • Ruby: 3.1.4

Additional context

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

@Jackson200 Jackson200 added the feature request A suggestion for a feature or enhancement label Feb 13, 2024
@allmarkedup
Copy link
Collaborator

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
Copy link
Author

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request A suggestion for a feature or enhancement
Projects
None yet
Development

No branches or pull requests

2 participants