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

chore: update to js-beautify v1.14.7 #1834

Closed
wants to merge 1 commit into from

Conversation

cexbrayat
Copy link
Member

The following commit in js-beautify introduced a breaking change for us, as custom element are now treated as inline elements beautifier/js-beautify@aa82eb9

This means a bunch of our tests, and snapshots of our users, break with this new version.

What do you think we should do?

  • lock the version of js-beautify for ever to v1.14.6
  • bump to v1.1.4.7, update our tests, and warn our users about the breaking change
  • talk about this with the js-beautify team and see if that should be configurable maybe?

The following commit in js-beautify introduced a breaking change for us, as custom element are now treated as inline elements
beautifier/js-beautify@aa82eb9

This means a bunch of our tests, and snapshots of our users, break with this new version.
@netlify
Copy link

netlify bot commented Oct 26, 2022

Deploy Preview for vue-test-utils-docs ready!

Name Link
🔨 Latest commit 1687d5f
🔍 Latest deploy log https://app.netlify.com/sites/vue-test-utils-docs/deploys/6359509cba251900089a14be
😎 Deploy Preview https://deploy-preview-1834--vue-test-utils-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@freakzlike
Copy link
Collaborator

I think we should talk with them to make it configurable. As it seems that all stubbed components are treat as custom elements

@cexbrayat
Copy link
Member Author

@freakzlike I started a conversation beautifier/js-beautify#2113

cexbrayat added a commit to cexbrayat/vue-test-utils-next that referenced this pull request Oct 27, 2022
until vuejs#1834 is resolved, we exclude js-beautify from renovate updates.
cexbrayat added a commit to cexbrayat/vue-test-utils-next that referenced this pull request Oct 27, 2022
Until vuejs#1834 is resolved, we exclude js-beautify from renovate updates.
cexbrayat added a commit that referenced this pull request Oct 27, 2022
Until #1834 is resolved, we exclude js-beautify from renovate updates.
@lmiller1990
Copy link
Member

Hi all, sorry about the slow replies, was on leave for a while and not coding much.

I agree we should try make it configurable. Snapshots are a bit unreliable (since implementation details like this can break them) but I'd really prefer to avoid breaking changes where possible. Looks like we've already got some movement in the JS Beautify repo, thanks @cexbrayat.

@lmiller1990
Copy link
Member

For now, should we just lock v1.14.6 and close this? The PR you made in beautify-web doesn't seem to have much movement.

@cexbrayat
Copy link
Member Author

@lmiller1990 We're locked on v1.14.6 (and I disabled Renovate for this package). I'll keep the PR around to have a reminder of this ongoing task, and hopefully we'll have feedback from the js-beautifier team soon (if not, then we'll close and stay on v1.14.6 forever...)

@JounQin
Copy link

JounQin commented Mar 24, 2023

Just curious, why js-beautify was chosen instead of prettier?

@lmiller1990
Copy link
Member

Just curious, why js-beautify was chosen instead of prettier?

We use it to format the output of html(). It's much smaller - prettier is a big dependency (it's like 15mb) which we don't need for what we are doing.

@nathanaelytj
Copy link

Hello, is this advisory related to this PR and issue?
GHSA-c2qf-rxjj-qqgw

Thank you

@cexbrayat
Copy link
Member Author

@nathanaelytj Not directly. We can't update to the latest js-beautify version because of a breaking change.
Regarding the vulnerability, if you only use js-beautify via test-utils, then you have nothing to worry about.

cexbrayat added a commit to cexbrayat/vue-test-utils-next that referenced this pull request Jul 20, 2023
The PR introducing a new option to _not_ inline custom elements has been merged and released,
allowing us to use the new version of js-beautify without introducing a breaking change.

See vuejs#1834 for context
cexbrayat added a commit to cexbrayat/vue-test-utils-next that referenced this pull request Jul 20, 2023
The PR introducing a new option to _not_ inline custom elements has been merged and released,
allowing us to use the new version of js-beautify without introducing a breaking change.

See vuejs#1834 for context
@cexbrayat
Copy link
Member Author

Superseeded by #2131

@cexbrayat cexbrayat closed this Jul 20, 2023
@cexbrayat cexbrayat deleted the chore/js-beautify-v1.14.7 branch July 20, 2023 14:38
cexbrayat added a commit that referenced this pull request Jul 20, 2023
The PR introducing a new option to _not_ inline custom elements has been merged and released,
allowing us to use the new version of js-beautify without introducing a breaking change.

See #1834 for context
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.

None yet

5 participants