Better explanation about the Prettier recommendation (extension vs. NPM module) #50629
Conversation
afharo
added
Team:Core
|
Pinging @elastic/kibana-platform (Team:Platform) |
💚 Build Succeeded |
CONTRIBUTING.md
Outdated
| @@ -325,7 +325,7 @@ Note that for VSCode, to enable "live" linting of TypeScript (and other) file ty | |||
| "eslint.autoFixOnSave": true, | |||
| ``` | |||
|
|
|||
| It is **not** recommended to use `prettier` plugin on Kibana project. Because settings are in `eslintrc.js` file and it is applied to too many files that shouldn't be prettier-ized. | |||
| :warning: It is **not** recommended to use the [`Prettier` extension/IDE plugin](https://prettier.io/) while maintaining the Kibana project. Formatting and styling roles are set in the multiple `.eslintrc.js` files across the project and some of them use the [NPM version of Prettier](https://www.npmjs.com/package/prettier). Using the IDE extension might cause conflicts, applying the formatting to too many files that shouldn't be prettier-ized. | |||
There was a problem hiding this comment.
Since I actually DO have Prettier enabled, and I've not noticed a problem, AND I'm pretty sure I've seen valid vscode errors labeled as coming from Prettier, that it's all that bad to have Prettier enabled. Also, especially since I don't believe I've run into a case where Prettier did it's thing when it shouldn't have.
I think it's worth keeping this in as a note though, since this seems to be common lore at this point, and I've not done any kind of exhaustive test on it. Definitely good to have some rationale as to why this is a warning, which we didn't have before.
There was a problem hiding this comment.
@pmuellr Thank you for your input, your experience with the plugin is very helpful. I've added the valid errors (yet Prettier highlighting them as wrong) to the line.
There was a problem hiding this comment.
In some part of the code, I had linting issues between what prettier generates and what eslint expects. I think it's not everywhere, but regarding anonymous functions, prettier and eslint had different spacing rules in some folders:
eslint:
const a = function () {}
prettier
const a = function() {}
So I actually had to change my intellij watcher from prettier to eslint
|
Should probably wait for another review before merging ... we've been requiring two reviews for Kibana Stack Services PRs, somehow now only one review seems like it's not enough. :-) |
afharo
added
backport
v7.4.3
v7.5.0
v7.6.0
and removed
backport:skip
|
Following @pmuellr recommendations, added the backport label |
💚 Build Succeeded |
…PM module) (elastic#50629) * Better explanation about the Prettier recommendation (extension vs. NPM module) * Contributing docs: Add side effect from using the Prettier extension
…-fallback * 'master' of github.com:elastic/kibana: (116 commits) [Maps] move apply global filter settting from layer to source (elastic#50523) [SIEM] Fix: Empty `Source` / `Destination` shown when only ports are populated (elastic#50843) [Maps] Delay vector tile layer syncing until spritesheet is loaded (elastic#48955) [Maps] prevent users from overflowing URL when filtering by shape (elastic#50747) [DOCS] Mark Beats central management as discontinued (elastic#49423) [page_objects/common_page] convert to ts (elastic#50771) [NP Kibana Migrations ] kibana plugin home (elastic#50444) [DOCS] Shareables naming convention (elastic#50497) [ML] DF Analytics - auto-populate model_memory_limit (elastic#50714) Increase alerting test stability and reduce flakiness (elastic#50246) [ML] Remaning new_job_new folder (elastic#50917) [Telemetry] Show opt-in changes for OSS users (elastic#50831) [ML] Fix lat_long anomalies table links menu and value formatting (elastic#50916) [Dev] Fix serialising a really big string (elastic#50915) Better explanation about the Prettier recommendation (extension vs. NPM module) (elastic#50629) [Monitoring] Use a basic monitoring user for tests (elastic#47865) [Monitoring] Gracefully handle issue with filebeat indices (elastic#48929) [Monitoring] Improve permissions required around setup mode (elastic#50421) Additional validation for elasticsearch username (elastic#48247) Revert changes to use_kibana_ui_setting (elastic#50877) ... # Conflicts: # src/legacy/core_plugins/console/server/request.test.ts
Summary
After the discussion in the Slack channel, extending the explanation about the Prettier recommendation in the contributing guidelines.
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.[ ] This was checked for cross-browser compatibility, including a check against IE11[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support[ ] Documentation was added for features that require explanation or tutorials[ ] Unit or functional tests were updated or added to match the most common scenarios[ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers