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

[UI] feat: Setup prettier and moved lint-staged to its own file #8827

Merged
merged 4 commits into from Sep 24, 2023

Conversation

nebula-aac
Copy link
Contributor

@nebula-aac nebula-aac commented Sep 19, 2023

Notes for Reviewers

This PR fixes #8661

Here is the proposed prettier config, where some of the existing formatting rules in eslint can be migrated to allow Prettier format the files for you.

arrowParens: always
bracketSameLine: false
bracketSpacing: true
jsxSingleQuote: false
proseWrap: preserve
quoteProps: as-needed
semi: true
singleQuote: true
tabWidth: 2
trailingComma: all
printWidth: 100
useTabs: false

I'm testing other rules, and may have removed something that you feel that it should be in the current config.

Please note that in order for this rules to work, you need to use next lint instead of eslint to lint the files, because eslint and next lint works differently from each other.

The lint-staged has been moved to its own config file for clarity and also takes care linting and formatting with the scripts available.

Feedback is welcomed, and this PR is by no means the definite PR, this can be improved with the configs.

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Antonette Caldwell <acald.nebula@gmail.com>
@github-actions github-actions bot added component/ui User Interface component/extensions Issues related to extension points labels Sep 19, 2023
@cypress
Copy link

cypress bot commented Sep 19, 2023

Passing run #1271 ↗︎

0 6 4 0 Flakiness 0

Details:

Merge 8ffded1 into 6640f6f...
Project: Meshery Commit: 07d473419b ℹ️
Status: Passed Duration: 00:56 💡
Started: Sep 24, 2023 5:17 PM Ended: Sep 24, 2023 5:18 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@nebula-aac nebula-aac requested review from a team September 20, 2023 22:45
Copy link
Member

@Abhishek-kumar09 Abhishek-kumar09 left a comment

Choose a reason for hiding this comment

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

The lint changes looks good. Since, I am not aware of the motivation behind due to lack of availability, I would comment less. But the new config looks good.

Yashsharma1911
Yashsharma1911 previously approved these changes Sep 23, 2023
"react/jsx-filename-extension" : [
plugins: ['react', 'cypress', 'prettier'],
rules: {
'@next/next/no-img-element': 'off',
Copy link
Contributor

Choose a reason for hiding this comment

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

@nebula-aac All looks good though I have a question should be doing same for meshery cloud as we are not using next Image there too, and getting bunch of lint error on that or we should not silence it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Yashsharma1911 I have it turned off now because in using Image from Next, it requires the width and height to be used, and some of the images we have, we're doing without. It would be a good idea to turn it off in Meshery Cloud too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Yashsharma1911 can you approve again? I updated the lint staged to fix the linting at commits

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure

Signed-off-by: Antonette Caldwell <134739862+nebula-aac@users.noreply.github.com>
Signed-off-by: Antonette Caldwell <134739862+nebula-aac@users.noreply.github.com>
Signed-off-by: Antonette Caldwell <134739862+nebula-aac@users.noreply.github.com>
@leecalcote
Copy link
Member

tabWidth: 2 - nice.


> **NOTE:** Its strongly recommended to use either [Node Version Manager](https://github.com/nvm-sh/nvm#node-version-manager---) in linux/mac os systems or [NVM for Windows](https://github.com/coreybutler/nvm-windows#nvm-for-windows) on Windows systems so single `nvm use` / `nvm install` simplifies installing and using correct node version locallly **(v18)**, see [NVM Intro](https://github.com/nvm-sh/nvm#intro) for details. Otherwise, you might experience issues during local `npm i` similar to [4674](https://github.com/meshery/meshery/issues/4674) due to how optional dependencies are resolved in npm v6.

> **NOTE:** Please run the steps in order to avoid issues, as Meshery server should be running and logged-in before accessing the development server
Copy link
Member

Choose a reason for hiding this comment

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

Could we please have these notes added to one or more of the contributing docs on docs.meshery.io?

@leecalcote
Copy link
Member

Updates needed in:

  1. UI Docs https://docs.meshery.io/project/contributing/contributing-ui
  2. Windows Docs - https://docs.meshery.io/project/contributing/meshery-windows

What instructions do we have or preconfigured settings can we share for popular IDEs like VSCode?

@nebula-aac
Copy link
Contributor Author

nebula-aac commented Sep 24, 2023

Updates needed in:

  1. UI Docs https://docs.meshery.io/project/contributing/contributing-ui
  2. Windows Docs - https://docs.meshery.io/project/contributing/meshery-windows

What instructions do we have or preconfigured settings can we share for popular IDEs like VSCode?

I can create a separate PR for this.

At this time, VS Code will pick up whatever settings is in the prettier config, but depending on the existing setup, it may get confusing. So we may need additional configs to sure this works properly and lessen confusion. Probably by way of using VSCode settings.

@nebula-aac nebula-aac merged commit 3903685 into master Sep 24, 2023
14 of 15 checks passed
@nebula-aac nebula-aac deleted the feat/setup-prettier branch September 24, 2023 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/extensions Issues related to extension points component/ui User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[UI] Setup Prettier
4 participants