-
Notifications
You must be signed in to change notification settings - Fork 1.3k
MCP SDK: Add Prettier to Typescript SDK #976
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
MCP SDK: Add Prettier to Typescript SDK #976
Conversation
Tests are failing because Edit: It got fixed on |
"bracketSpacing": true, | ||
"bracketSameLine": false, | ||
"proseWrap": "always", | ||
"arrowParens": "avoid", |
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.
could you add as well the sort imports plugin while at it ? (you need to add @trivago/prettier-plugin-sort-imports
as a dev dependency as well)
"arrowParens": "avoid", | |
"arrowParens": "avoid", | |
"plugins": ["@trivago/prettier-plugin-sort-imports"], | |
"importOrder": ["<THIRD_PARTY_MODULES>", "^../", "^./"], | |
"importOrderSortSpecifiers": true, | |
"importOrderSeparation": true, |
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.
There's an eslint plugin for that I believe rather than prettier.
I also suggest introducing changes in an iterative matter, more can be done after this PR - there's further linting rules we could benefit from.
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.
Understood - better via eslint and in a later stage. Wanted to avoid having again a big diff introducing a new rule (that will impact all files for sure). Can't wait for this to be merged ❤️
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 so much!
will merge closer to the package release day (Thursday) to get in a few more PRs without merge conflicts
This PR's aim is to introduce Prettier to the Typescript SDK, in order to have consistent formatting.
It is a follow up to #953 and effectively closing it. This PR contains the prettier config changes only so maintainers can run
npx prettier --write .
on the full repo.Motivation and Context
Formatting is not automatic and there is no code formatting/styling tool to standardize that. That leads to PR code reviews having to manually identify any styling issues as opposed to that being automatic.
It is integrating prettier with the existing ESlint set up, and adding prettier --check . to the "lint" command in package.json
How Has This Been Tested?
Ran prettier --check . and prettier --write . (to apply the styling) on the full repository.
Breaking Changes
None, it's a devDependency.
Types of changes
Checklist
Additional context
As a further step, if Prettier gets agreed on and merged, a few improvements could be done on this initiative: