cboard-org
Add TS files to Prettier formatting - Issue #1896
Per discussion in #1896 this update enables Prettier on .ts and .tsx files.
The previous version of Prettier was quite outdated so this update bumps that version, and removes the redundant git add from the lint-staged hook that hasn't been needed since v10.
As I mentioned in the issue, I can also introduce some rules in .prettierrc to approximate previous behavior and reduce the number of changes when committing updates moving forward, if desired.
Hi @zkarmi! This is awesome—thank you! I just saw the issue comment you left. I agree, we should implement the suggested changes in the .prettierrc file. Otherwise, too many formatting changes are introduced whenever an old file is saved.
I'll check with @tomivm about how we want to handle reformatting existing files. My initial thought is to create a separate commit for formatting changes in upcoming PRs. Let me know if you have any suggestion
@RodriSanchez1 Glad I can help. I'm adding a commit for the expanded .prettierrc. I'm swapping out "jsxBracketSameLine": false for "bracketSameLine": false, as the former is deprecated.
Are you suggesting to first commit to apply formatting to the file, then make another commit with actual changes? That could make things cleaner when reviewing PRs, and you wouldn't have to turn off lint-staged. It would mean you have to introduce that into the PR workflow, though.
Are you suggesting to first commit to apply formatting to the file, then make another commit with the actual changes? That could make things cleaner when reviewing PRs, and you wouldn't have to turn off lint-staged.
Yes, that's exactly what I had in mind.
It would mean you have to introduce that into the PR workflow, though.
That's a good point. I'll think about whether there's a better way to handle this. But either way, we'll need to push the new formatting at some point., so it might be cleaner to address it proactively.
Hey @zkarmi!
Today we had the Cboard weekly dev meeting and talked about the formatting modifications. We concluded that we are going to implement them in one PR, probably this one, where the whole project will be reformatted.
Next steps:
I'm going to do further research and check if there's any other property we can add to the .prettierrc file. I know you already did that, but if there's anything you think I should look into or any documentation I should check, let me know.
We also need to double-check what will happen with the open PRs. Are we going to face any problems or conflicts if we apply the new formatting? If that's the case, the best would be to close as many PRs as possible before merging this one.
Thanks for the update, @RodriSanchez1 !
I've not done such a Prettier migration myself previously, but from some research online the general consensus I've seen has been to do it all in one big PR, as you said. And yes, it seems like suggestions are to wrap up PRs and not create new ones while updating, to avoid merge conflicts.
While doing some reading I did come across this article where the author's team used some scripts to make the changes, allowing the git blame to fall on a tool rather than a specific dev. Maybe it will be helpful to the team, I wanted to share because I haven't had time to dive deep on it myself: https://flexport.engineering/upgrading-prettier-on-a-large-codebase-28d56c4de49e.
Just to add here -- I would recommend reviewing which props were added after 1.15. The docs do show which version an option first appeared in: https://prettier.io/docs/options
That's awesome! Thanks for sharing. The team was right hehe. I will be checking out this week!