-
Notifications
You must be signed in to change notification settings - Fork 668
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
Issue(1147): Add option to pretty print html components #1229
Issue(1147): Add option to pretty print html components #1229
Conversation
87c79a9
to
23dc630
Compare
3665be0
to
e1ff1e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes! I've been waiting for this 🥇
Thank you for the PR, as per my comment in the issue: can you make pretty-printing the default behavior? |
@eddyerburgh done! I have updated the existing test to use the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool feature at a relatively low cost. Thanks for the initiative @addisonssense !
@addisonssense I think existing tests should be updated with Just my $0.02 ✌️ But anyhow, nice PR! |
@briwa I think you make a good point. I'll do that 👍 |
A few months ago I proposed a different approach to this. TLDR:
This allows us to change the defaults again in the future without worrying about breaking people's tests, as the defaults can be documented for each version. |
I intend to implement the changes in jest-serializer-vue but I haven't had time to work on it. jest-serializer-vue will be formatted more heavily, whereas vue-test-utils can just use the defaults. @addisonssense thanks for updating, but I wasn't clear in my previous comment. I don't think there should be an option to pretty-print or not. I don't think it should be configurable, and pretty printing should always be the default behavior. Can you update the PR to remove the |
@eddyerburgh done! Pretty printing is now the default. The option to disable it is removed. |
9e6fc10
to
72f10fb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks :)
Hey @addisonssense, thank you for your time and effort spent on this PR, contributions like yours help make Vue better for everyone. Cheers! 💚 |
This PR is to implement pretty printing for html components.
Fixes #1147