From b03ad7072cdb234eb06f3dc3ac4fc635d130e971 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 18 May 2020 14:14:09 -0400 Subject: [PATCH 1/9] =?UTF-8?q?=F0=9F=92=85=20Add=20Prettier=20?= =?UTF-8?q?=F0=9F=92=87?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- text/0628-prettier.md | 135 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 text/0628-prettier.md diff --git a/text/0628-prettier.md b/text/0628-prettier.md new file mode 100644 index 0000000000..a37b7d18cb --- /dev/null +++ b/text/0628-prettier.md @@ -0,0 +1,135 @@ +- Start Date: 2020-05-18 +- Relevant Team(s): Ember CLI +- RFC PR: https://github.com/emberjs/rfcs/pull/628 +- Tracking: (leave this empty) + +# Add Prettier + +## Summary + +This RFC proposes adding [Prettier](https://prettier.io/) to the blueprints +that back `ember new` and `ember addon`. + +## Motivation + +Using Prettier removes the ever present and ongoing stylistic debates that tend +to happen on teams. Prettier is incredibly freeing for both developers **and** +reviewers of a codebase. The developer can author in whatever format they find +easiest to type out, avoiding needless wasted time trying to get that +indentation _just_ right, and either setup an automated "format on save" +operation in their editor or use a quick command line invocation to `--fix` all +files at once. + +This dove tails very nicely with Embers deeply ingrained goal of using shared +solutions to solve common problems, and aligns very well with the general trend +to play better with the general JavaScript ecosystem. + +## Detailed design + +Due to the pre-existing design (and improvements added to the blueprints in +[ember-cli/rfcs#121](https://github.com/emberjs/rfcs/blob/master/text/0121-remove-ember-cli-eslint.md)) +implementation is very straight forward. The general idea is that we will +update the `app` and `addon` blueprints to add a few Prettier related packages +to the `package.json`, update the linting configuration to enforce Prettier +styles, and update the other blueprint code to ensure that its output is +Prettier compatible. + +### Packages + +The following packages will be added to the `package.json` of both `app` and `addon` blueprints: + +* [prettier](https://www.npmjs.com/package/prettier) - The main package (used by the other packages). +* [eslint-config-prettier](https://www.npmjs.com/package/eslint-config-prettier) - Disables stylistic ESLint rules that would conflict with the Prettier output. +* [eslint-plugin-prettier](https://www.npmjs.com/package/eslint-plugin-prettier) - Enables `eslint-config-prettier`s `recommended` set, and adds the `prettier/prettier` rule to enforce styles. + +### Configuration Changes + +#### ESLint + +The `.eslintrc.js` that is generated will be updated to add: + +```js +{ + extends: ['plugin:prettier/recommended'], +} +``` + +#### Prettier + +A new `prettier` section will be added to the `package.json` file with the following contents: + +```json +{ + "prettier": { + "singleQuotes": true + } +} +``` + +There is a [small number of configuration +options](https://prettier.io/docs/en/options.html) that can be used in this +file, but we are intentionally avoiding modifying the default values for things +that are not _nearly_ universally agreed on in the Ember ecosystem. + +### Blueprint Changes + +In general, it is recommended that all blueprints provided by addons should +satisfy the default linting configuration of a new Ember application. As such +the blueprints provided by `ember-cli`, `ember-source`, and `ember-data` will +be updated to ensure that they satisfy these new linting rules requirements. + +#### `package.json` scripts + +The `app` and `addon` blueprints will be updated to add the following additional entries to `scripts`: + +```json +{ + "scripts": { + "lint:fix": "npm-run-all --aggregate-output --continue-on-error --parallel lint:*:fix", + "lint:js:fix": "eslint . --fix", + "lint:hbs:fix": "ember-template-lint . --fix" + } +} +``` + +This configuration is specifically intending to allow users to add additional linters +(e.g. `stylelint` or `markdownlint`) by adding scripts for them, and they would +automatically be rolled up into `lint:fix`. + +### Codemod + +In order to make the migration as simple as possible, a codemod will be created: `ember-cli-prettier-codemod`. + +The codemod will do the following: + +* Install the new/required dependencies +* Migrate an application / addon to the new linting configuration +* Commit _just_ the configuration changes. +* Run the new `lint:fix` script to update all files to the new format. +* Commit just the auto-fixed output. + +While this seems quite simple (and in fact it is pretty easy), leveraging a +codemod will make it much easier to deal with rebasing large Prettier migration +pull requests. If one of those pull requests gets out of date (e.g. due to a +conflict in one of the files) you can simply re-run the codemod on the branch +and it will replace the previously created commits. + +## How we teach this + +We do not currently discuss linting or stylistic formatting in either guides.emberjs.com or cli.emberjs.com. + +This RFC proposes adding a new subsection that discusses linting under [`Basic Use` section of the CLI guides](https://cli.emberjs.com/release/basic-use/). + +## Drawbacks + +The largest drawback is generally the cost of the _initial_ migration to Prettier. That migration tends to be quick, but it can cause pain for folks maintaining long lived branches. + +## Alternatives + +Do nothing. + +## Unresolved questions + +> In general, users of `yarn` could be perfectly happy with `yarn lint:js --fix` / `yarn lint:hbs --fix` (which works today). Should we still add the `lint:js:fix`/`lint:hbs:fix` scripts? + +I went with "yes" here in the initial RFC prose, as I'd general prefer to _reduce_ the differences between users of `npm` and `yarn` over time. From a32ad2d069ff54a31e70c4cdae9e75baa8f430b8 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 18 May 2020 17:47:35 -0400 Subject: [PATCH 2/9] Ensure `yarn lint` does not run `lint:fix`. --- text/0628-prettier.md | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index a37b7d18cb..d30a16f47e 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -80,21 +80,26 @@ be updated to ensure that they satisfy these new linting rules requirements. #### `package.json` scripts -The `app` and `addon` blueprints will be updated to add the following additional entries to `scripts`: +The `app` and `addon` blueprints will be updated to add the following +additional entries to `scripts`: ```json { "scripts": { + "lint": "npm-run-all --aggregate-output --continue-on-error --parallel 'lint:!(fix)'", "lint:fix": "npm-run-all --aggregate-output --continue-on-error --parallel lint:*:fix", "lint:js:fix": "eslint . --fix", "lint:hbs:fix": "ember-template-lint . --fix" } } ``` +The `lint:fix`, `lint:js:fix`, `lint:hbs:fix` scripts are new (introduced with +this RFC), and the `lint` script will be updated to ensure that it avoids +running the new `lint:fix`. -This configuration is specifically intending to allow users to add additional linters -(e.g. `stylelint` or `markdownlint`) by adding scripts for them, and they would -automatically be rolled up into `lint:fix`. +This configuration is specifically intending to allow users to add additional +linters (e.g. `stylelint` or `markdownlint`) by adding scripts for them, and +they would automatically be rolled up into `lint:fix`. ### Codemod From fa51038914fcae89302530bddd077f11323d48c5 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Mon, 18 May 2020 17:53:24 -0400 Subject: [PATCH 3/9] Fix typo (singleQuote not singleQuotes). --- text/0628-prettier.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index d30a16f47e..25836fd721 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -61,7 +61,7 @@ A new `prettier` section will be added to the `package.json` file with the follo ```json { "prettier": { - "singleQuotes": true + "singleQuote": true } } ``` From ecf524de4166be1c4892f2250c3f749b5cd0524e Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 22 May 2020 19:18:53 -0400 Subject: [PATCH 4/9] Add `eslint --cache` to `lint:js`. --- text/0628-prettier.md | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index 25836fd721..a5c6911785 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -71,6 +71,12 @@ options](https://prettier.io/docs/en/options.html) that can be used in this file, but we are intentionally avoiding modifying the default values for things that are not _nearly_ universally agreed on in the Ember ecosystem. +#### Git + +The `.gitignore` will be updated to add `.eslintcache` so that when using +`eslint --cache` the cache file itself won't be added to the repositories +history. + ### Blueprint Changes In general, it is recommended that all blueprints provided by addons should @@ -88,14 +94,18 @@ additional entries to `scripts`: "scripts": { "lint": "npm-run-all --aggregate-output --continue-on-error --parallel 'lint:!(fix)'", "lint:fix": "npm-run-all --aggregate-output --continue-on-error --parallel lint:*:fix", + "lint:js": "eslint . --cache", "lint:js:fix": "eslint . --fix", "lint:hbs:fix": "ember-template-lint . --fix" } } ``` -The `lint:fix`, `lint:js:fix`, `lint:hbs:fix` scripts are new (introduced with -this RFC), and the `lint` script will be updated to ensure that it avoids -running the new `lint:fix`. + +A few callouts here: + +* The `lint:fix`, `lint:js:fix`, `lint:hbs:fix` scripts are new (introduced with this RFC) +* The `lint` script will be updated to ensure that it avoids running the new `lint:fix` script +* The `lint:js` script will be updated to add caching This configuration is specifically intending to allow users to add additional linters (e.g. `stylelint` or `markdownlint`) by adding scripts for them, and From bfd0c05727bb3f86c9fc3258460cddf9aa3d2b29 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 22 May 2020 19:23:20 -0400 Subject: [PATCH 5/9] Add standardjs.com as a plausible alternative --- text/0628-prettier.md | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index a5c6911785..789080e4d3 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -141,7 +141,23 @@ The largest drawback is generally the cost of the _initial_ migration to Prettie ## Alternatives -Do nothing. +> Why not adopt [standard](https://standardjs.com/) instead of Prettier? + +Prettier is **much** more popular with [9,249,847 weekly +downloads](https://www.npmjs.com/package/prettier) vs [265,658 weekly +downloads](https://www.npmjs.com/package/standard) (we all know these download +numbers don't mean a ton, but the order of magnitude can tell us something), +and (aside from the defaulting of `singleQuotes`) is much more aligned with the +Ember ecosystems inherent preferences. + +Additionally, a very large number of the Ember ecosystem +projects are _already_ using Prettier internally. These include `ember-source`, +`ember-cli`, `ember-data`, `@ember/test-helpers`, `eslint-plugin-ember`, the +various packages composing the rendering engine, `@glimmer/component`, +`ember-template-lint`, and many more. Additionally a large number of community +maintained addons are also using it already ([here is a +listing](https://emberobserver.com/code-search?codeQuery=prettier&fileFilter=package.json&sort=score&sortAscending=false) +using EmberObservers awesome code search feature). ## Unresolved questions From cc4865d1f8d34faaa70c694909c128b1cf5cd63d Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Fri, 22 May 2020 19:25:07 -0400 Subject: [PATCH 6/9] Ensure guides are audited for prettified content --- text/0628-prettier.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index 789080e4d3..80ff96e382 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -133,7 +133,12 @@ and it will replace the previously created commits. We do not currently discuss linting or stylistic formatting in either guides.emberjs.com or cli.emberjs.com. -This RFC proposes adding a new subsection that discusses linting under [`Basic Use` section of the CLI guides](https://cli.emberjs.com/release/basic-use/). +This RFC proposes adding a new subsection that discusses linting under [`Basic +Use` section of the CLI guides](https://cli.emberjs.com/release/basic-use/). In +addition, when this RFC is implemented we should do a throughough audit of the +`cli.emberjs.com` and `guides.emberjs.com` content to ensure that all code +snippets are formatted correctly (e.g. using the Prettier specific formatting +introduced here). ## Drawbacks From 127619903757a41df84db318cb24b1d9fbdfe163 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Tue, 26 May 2020 10:07:01 -0400 Subject: [PATCH 7/9] Migrate to stand alone configuration files. --- text/0628-prettier.md | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index 80ff96e382..03ce5217f3 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -56,14 +56,12 @@ The `.eslintrc.js` that is generated will be updated to add: #### Prettier -A new `prettier` section will be added to the `package.json` file with the following contents: +A `.prettierrc.js` will be added with the following contents: -```json -{ - "prettier": { - "singleQuote": true - } -} +```js +module.exports = { + singleQuote: true, +}; ``` There is a [small number of configuration @@ -71,6 +69,31 @@ options](https://prettier.io/docs/en/options.html) that can be used in this file, but we are intentionally avoiding modifying the default values for things that are not _nearly_ universally agreed on in the Ember ecosystem. +A `.prettierignore` will be added to match the `.eslintignore` contents: + +``` +# unconventional js +/blueprints/*/files/ +/vendor/ + +# compiled output +/dist/ +/tmp/ + +# dependencies +/bower_components/ +/node_modules/ + +# misc +/coverage/ +!.* + +# ember-try +/.node_modules.ember-try/ +/bower.json.ember-try +/package.json.ember-try +``` + #### Git The `.gitignore` will be updated to add `.eslintcache` so that when using From 6ceceba94ffe22981f66a62b1b4f826c25f20376 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 27 May 2020 13:22:28 -0400 Subject: [PATCH 8/9] Add `lint:fix` upon blueprint generation. --- text/0628-prettier.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index 03ce5217f3..f107546ea1 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -107,6 +107,14 @@ satisfy the default linting configuration of a new Ember application. As such the blueprints provided by `ember-cli`, `ember-source`, and `ember-data` will be updated to ensure that they satisfy these new linting rules requirements. +In order to ensure blueprint output follows each individual projects custom +stylistic linting settings as much as possible. The following will be updated +to run `lint:fix` (when available) after their existing functionality: + +* `ember generate` +* `ember init` +* `ember-cli-update` + #### `package.json` scripts The `app` and `addon` blueprints will be updated to add the following From c48c3817b87d99972f4fa0251d9acea92aed4f32 Mon Sep 17 00:00:00 2001 From: Robert Jackson Date: Wed, 27 May 2020 13:22:50 -0400 Subject: [PATCH 9/9] Make drawback a block quote. --- text/0628-prettier.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/text/0628-prettier.md b/text/0628-prettier.md index f107546ea1..4140aeb904 100644 --- a/text/0628-prettier.md +++ b/text/0628-prettier.md @@ -173,7 +173,7 @@ introduced here). ## Drawbacks -The largest drawback is generally the cost of the _initial_ migration to Prettier. That migration tends to be quick, but it can cause pain for folks maintaining long lived branches. +> The largest drawback is generally the cost of the _initial_ migration to Prettier. That migration tends to be quick, but it can cause pain for folks maintaining long lived branches. ## Alternatives