feat(config-ui): added prettier to help format code#2995
Conversation
|
@mintsweet Prettier will cause conflicts with ESLint, especially with regard to single quotes use and JSX files. There is a method to have ESLint working together with prettier but it requires more configuration. When Prettier auto-formats JSX it will replace all single quotes with double quotes, ignoring the single quote rule altogether making it a requirement to run eslint again to backfix the double quotes. While prettier can be used locally for now, I don't want prettier formally checked into the codebase auto-formatting files until it has been properly tested & configured to operate with ESLint & JSX. |
|
My current local settings for Prettier+ESLint, I may increase 80 to 120 as far as print width goes. Prettier RC ESLint RC |
|
@e2corporation Prettier has a property jsxSingleQuote that fixes the problem you said. Everyone's code formatting style is different, using prettier is to ensure that the style is consistent under the same repository. I also added eslint-prettier. |
|
@e2corporation I observed that the max-len configured by eslint is 140, so I also set the printWidth of the prettier to 140. |
|
@mintsweet The jsxSingleQuote is partially deprecated and even though it is set to |
|
@mintsweet This PR can be planned for the next upcoming milestone, not |
|
@mintsweet Our UI stack also prefers no-semicolons. Please adjust your local settings accordingly so semicolons are not added to JS or JSX Files |
|
@e2corporation I added |
|
@mintsweet What is your primary Editor/IDE? Your Editor should be configured so configurations for Prettier and ESLint are read from the devlake project's config-ui directory, this way your global editor settings or other project specific settings are unaffected. These configuration files can exist on your local for now without checking them into the codebase and that should still allow you the ability to format. I have to complete other milestone tasks before shifting focus to properly review this ticket and test locally before merging. |
|
@e2corporation vscode. |
|
According to what I learned from @mintsweet, the configuration of this PR is compatible with the current codebase. The plan is:
If I understand it correctly, this PR should be merged ASAP:
@e2corporation I think we should merge this one first, what do you think? |
|
@klesh While it's good to have prettier configuration added to make thing easier, the point I would like to make is that the Developer is responsible for following the project's coding standards. Having these files checked in should not be needed or should not be preventing @mintsweet from completing his work on Webhooks. These are the minimal coding requirements to work on DevLake's UI, regardless of what Editor/IDE is being used. Aside from these basic rules, ESLint will ensure all files are meeting current spec. (This should be added to the UI readme)
This PR also adds |
|
@klesh @mintsweet This PR needs a rebase. PR Description also needs an update. |
e2corporation
left a comment
There was a problem hiding this comment.
LGTM (after rebase)
pr-type/bug-fix,pr-type/feature-development, etc.Summary
Does this close any open issues?
No.
Screenshots
Not any.
Other Information
Ensure the consistency of multi-person collaborative code style.