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

refactor: Remove deprecated concat #334

Merged

Conversation

carmanchris31
Copy link
Contributor

@carmanchris31 carmanchris31 commented Dec 11, 2022

builders.doc.concat is currently deprecated and is removed in prettier v3 (see https://github.com/prettier/prettier/wiki/How-to-migrate-my-plugin-to-support-Prettier-v3%3F)

This proactively removes it's usage so there are fewer changes required to support v3

edit: this does the same as #276 except against the latest code which also includes replacing the groupConcat method introduced in #313

I also see from that thread that we plan on doing this for v3 only. In that case, if you could create a next branch for v3 I could go ahead and start making pull requests for that as well. I've got a number of the changes figured out; I just wanted to see if there were any we could do ahead of time.

@dummdidumm
Copy link
Member

dummdidumm commented Dec 11, 2022

Thank you very much for tackling this, and for your desire to also work on some of the other changes!
As you noted, we can remove concat only for the the next major of prettier-plugin-svelte, so a next branch is the right approach. I'll create another PR with the next branch so it exists, and you can reroute your PR to that branch.

@carmanchris31
Copy link
Contributor Author

Perfect, thanks!

@carmanchris31 carmanchris31 changed the base branch from master to next December 12, 2022 00:34
@carmanchris31
Copy link
Contributor Author

Updated the base branch 👌

@carmanchris31
Copy link
Contributor Author

I noticed I missed a spot with some parts that still check for the Concat nodes.

@carmanchris31
Copy link
Contributor Author

Updated again :)

@carmanchris31
Copy link
Contributor Author

If we merge this in I can work on the next set of changes.

Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long - thank you! If you like you can continue to work on the next set of changes.

@dummdidumm dummdidumm merged commit e8aaeb8 into sveltejs:next Mar 15, 2023
@carmanchris31
Copy link
Contributor Author

Sorry for taking so long - thank you! If you like you can continue to work on the next set of changes.

Thanks for following up!

On the bright side, prettier agreed to ship their own types which is a huge help: prettier/prettier#14033

@carmanchris31 carmanchris31 deleted the refactor/remove-deprecated-concat branch March 19, 2023 18:08
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

2 participants