add prettier to format js files #1418

zhaozhiming · 2018-02-08T06:05:27Z

add prettier, lint-stage, husky and eslint-config-prettier
add git precommit hook, when commit will use prettier to format git add code
updated lint-fix script to use prettier to format project js code
use prettier to format existed code

* Update webpack.config.renderer.dev.dll.js * update webpack to support sass files * update webpack to support sass files

amilajack · 2018-02-08T06:40:27Z

Good stuff! 🎉 Will merge this in soon after some testing

amilajack · 2018-02-08T06:41:30Z

Can you update the CHANGELOG with the changes you made? Let's push this under v0.14.0

zhaozhiming · 2018-02-08T06:45:35Z

OK. I will update it. It's my pleasure.

zhaozhiming · 2018-02-08T07:12:24Z

I have not idea which changelog is better.
@amilajack Could you give some suggestions?

A

# 0.13.3 (2018.2.8)
- Add git precommit hook, when commit will use prettier to format git add code
- Add `format` npm script which use prettier to format project js code

B

# 0.13.3 (2018.2.8)
- Add prettier to format js files

amilajack · 2018-02-08T07:12:46Z

A is better. Also I think the lint and format scripts should be the same. lint-fix should autofix the files. lint should just lint them and show the errors

zhaozhiming · 2018-02-08T07:55:00Z

I combed the scripts again.
And I found prettier is default ignore node_modules so no need to write '*/!(node_modules)/**/*.js' in script.
Please review continue. Thanks.

amilajack · 2018-02-08T17:58:12Z

package.json

@@ -12,6 +12,8 @@
    "electron-rebuild": "electron-rebuild --parallel --force --types prod,dev,optional --module-dir app",
    "flow": "flow",
    "flow-typed": "rimraf flow-typed/npm && flow-typed install --overwrite || true",
+    "format": "prettier --single-quote --list-different '**/*.js'",


I think the "format" script should be removed and the "format-fix" should be merged into the "lint-fix" script.

This will make it easier for existing users to upgrade without changing their workflow

…cripts

….com for new add package

…ch files need to format

zhaozhiming · 2018-02-09T02:14:44Z

@amilajack I have done the request change. Please review. Thanks.
@BuckyMaler @ziedhajsalah Hi, I have resolve the conflicts with your code. Could you review my merge code? If I broke something, please tell me. Thanks.

amilajack · 2018-02-10T07:33:52Z

Thanks! I will review this soon and merge

amilajack · 2018-02-13T23:15:51Z

package.json

@@ -31,6 +32,12 @@
    "test-watch": "npm test -- --watch"
  },
  "browserslist": "electron 1.6",
+  "lint-staged": {
+    "*.js": [
+      "prettier --single-quote --write",


Shouldn't lint-staged auto fix with eslint as well?

Yes, it can add eslint --fix.
But I think what eslint --fix can do as same as prettier(something like add semicolon or double quotes to single quote) .
I don't known what error eslint can auto-fix but prettier can't.

This is the eslint official doc said, but I don't understand what it mean.

If eslint --fix has the same effect as prettier, I think keep only prettier is better.

amilajack · 2018-02-13T23:17:32Z

package.json

@@ -13,14 +13,15 @@
    "flow": "flow",
    "flow-typed": "rimraf flow-typed/npm && flow-typed install --overwrite || true",
    "lint": "cross-env NODE_ENV=development eslint --cache --format=node_modules/eslint-formatter-pretty .",
-    "lint-fix": "npm run lint -- --fix",
+    "lint-fix": "npm run lint -- --fix && prettier --single-quote --write '**/*.js'",


We're linting **/*.js but files in the **/**/*.js glob are linted as well. Hmm interesting. For example, app/components/*.js files are listed. Just wondering why this is

Prettier use glob syntax from the glob module.
** matches zero or more directories and subdirectories.

The following characters have special magic meaning when used in a path portion:

...

** If a "globstar" is alone in a path portion, then it matches zero or more directories and subdirectories searching for matches. It does not crawl symlinked directories.

Maybe put the prettier before eslint --fix is better? What do you think?

Well if eslint auto fixes through the prettier eslint config then shouldn't we just need to eslint auto fix? Not sure 100% how this works.

I think eslint --fix and prettier have some different. There is a example show the different in this discuss.

Now I think the original way is better. Use eslint --fix first and then use prettier to format fixed code. Please forgive my capricious personality.

"lint-fix": "npm run lint -- --fix && prettier --single-quote --write '**/*.js'",

Sounds good 👍

* Misc code style changes to menu.js * v0.13.3 * More consistent node path * Allowed node_modules to be checked by flow * add prettier to format js files (#1418) * Remove jsdom dep (#1411) * Remove dynamic import dep (#1408) * Add .sass files support (#1412) * Update webpack.config.renderer.dev.dll.js * update webpack to support sass files * update webpack to support sass files * Misc code style changes to menu.js * chore: add perttier husky lint-stage eslint-config-prettier and add scripts * refactor: use prettier to format code * fix: fix lint error and add eslint prettier config * fix: replace registry url from registry.npmjs.org to registry.yarnpkg.com for new add package * docs: update changelog * chore: add format-fix script and make format script just to check which files need to format * format: use prettier to format webpack files * docs: update change log - add format-fix script * feat: add prettier in `lint-fix` script * Removed unnecessary deps * Updated deps * Upgraded to webpack 4 * Run prettier even if eslint fails * createBrowserHistory to createHashHistory for prod (#1184) * Filter deps without entrypoint from dll * Bumped deps * Temporary hack to get flow working with webpack-cli * Use module property from dev webpack config in DLL webpack config (#1497) * Remove jsdom dep (#1411) * Remove dynamic import dep (#1408) * Add .sass files support (#1412) * Update webpack.config.renderer.dev.dll.js * update webpack to support sass files * update webpack to support sass files * Misc code style changes to menu.js * Use module property from dev webpack config in DLL webpack config Fixes #1468 To avoid a circular dependency, this relies on a check against the parent module's filename for `webpack.config.renderer.dev.dll.js`. It's expected that developers who rename config files will hopefully grep the codebase for this filename before changing it. This change also includes this check again when configuring `DllReferencePlugin`, because it is no longer guaranteed that the DLL manifest file exists by the time the renderer config specifies plugins. I played around with the idea of creating a `webpack-merge` strategy that plucks the value of `module` from the dev webpack config, but it wouldn't help much because, in `webpack.config.renderer.dev.js`, the `DllReferencePlugin` would still try to require the manifest (which would not exist). * Use includes() rather than indexOf() * Updated all deps to latest semver * Bumped deps * Bumped all deps to latest semver * Update changelog * Increased delay for e2e counter test * Updated lock file * Bumped ci node versions * Reverted version change in CHANGELOG [ci skip]

* Misc code style changes to menu.js * v0.13.3 * More consistent node path * Allowed node_modules to be checked by flow * add prettier to format js files (electron-react-boilerplate#1418) * Remove jsdom dep (electron-react-boilerplate#1411) * Remove dynamic import dep (electron-react-boilerplate#1408) * Add .sass files support (electron-react-boilerplate#1412) * Update webpack.config.renderer.dev.dll.js * update webpack to support sass files * update webpack to support sass files * Misc code style changes to menu.js * chore: add perttier husky lint-stage eslint-config-prettier and add scripts * refactor: use prettier to format code * fix: fix lint error and add eslint prettier config * fix: replace registry url from registry.npmjs.org to registry.yarnpkg.com for new add package * docs: update changelog * chore: add format-fix script and make format script just to check which files need to format * format: use prettier to format webpack files * docs: update change log - add format-fix script * feat: add prettier in `lint-fix` script * Removed unnecessary deps * Updated deps * Upgraded to webpack 4 * Run prettier even if eslint fails * createBrowserHistory to createHashHistory for prod (electron-react-boilerplate#1184) * Filter deps without entrypoint from dll * Bumped deps * Temporary hack to get flow working with webpack-cli * Use module property from dev webpack config in DLL webpack config (electron-react-boilerplate#1497) * Remove jsdom dep (electron-react-boilerplate#1411) * Remove dynamic import dep (electron-react-boilerplate#1408) * Add .sass files support (electron-react-boilerplate#1412) * Update webpack.config.renderer.dev.dll.js * update webpack to support sass files * update webpack to support sass files * Misc code style changes to menu.js * Use module property from dev webpack config in DLL webpack config Fixes electron-react-boilerplate#1468 To avoid a circular dependency, this relies on a check against the parent module's filename for `webpack.config.renderer.dev.dll.js`. It's expected that developers who rename config files will hopefully grep the codebase for this filename before changing it. This change also includes this check again when configuring `DllReferencePlugin`, because it is no longer guaranteed that the DLL manifest file exists by the time the renderer config specifies plugins. I played around with the idea of creating a `webpack-merge` strategy that plucks the value of `module` from the dev webpack config, but it wouldn't help much because, in `webpack.config.renderer.dev.js`, the `DllReferencePlugin` would still try to require the manifest (which would not exist). * Use includes() rather than indexOf() * Updated all deps to latest semver * Bumped deps * Bumped all deps to latest semver * Update changelog * Increased delay for e2e counter test * Updated lock file * Bumped ci node versions * Reverted version change in CHANGELOG [ci skip]

BuckyMaler and others added 4 commits February 2, 2018 21:09

Remove jsdom dep (electron-react-boilerplate#1411)

9febffb

Remove dynamic import dep (electron-react-boilerplate#1408)

03f7691

Add .sass files support (electron-react-boilerplate#1412)

06ce8c4

* Update webpack.config.renderer.dev.dll.js * update webpack to support sass files * update webpack to support sass files

Misc code style changes to menu.js

586b84f

amilajack self-requested a review February 8, 2018 07:14

amilajack changed the base branch from master to v0.13.3 February 8, 2018 17:52

amilajack requested changes Feb 8, 2018

View reviewed changes

zhaozhiming added 8 commits February 9, 2018 09:36

chore: add perttier husky lint-stage eslint-config-prettier and add s…

d824487

…cripts

refactor: use prettier to format code

feafa3a

fix: fix lint error and add eslint prettier config

b03c77c

fix: replace registry url from registry.npmjs.org to registry.yarnpkg…

d675516

….com for new add package

docs: update changelog

53b5825

chore: add format-fix script and make format script just to check whi…

c9cc9e3

…ch files need to format

format: use prettier to format webpack files

0d11174

docs: update change log - add format-fix script

c310606

zhaozhiming force-pushed the prettier branch from 17dcb19 to 033740d Compare February 9, 2018 01:41

zhaozhiming added 2 commits February 9, 2018 09:42

feat: add prettier in lint-fix script

033740d

Merge branch 'v0.13.3' into prettier

85533d0

amilajack requested changes Feb 13, 2018

View reviewed changes

amilajack approved these changes Feb 22, 2018

View reviewed changes

amilajack merged commit 482dddd into electron-react-boilerplate:v0.13.3 Feb 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add prettier to format js files #1418

add prettier to format js files #1418

zhaozhiming commented Feb 8, 2018 •

edited by amilajack

amilajack commented Feb 8, 2018

amilajack commented Feb 8, 2018

zhaozhiming commented Feb 8, 2018

zhaozhiming commented Feb 8, 2018

amilajack commented Feb 8, 2018 •

edited

zhaozhiming commented Feb 8, 2018

amilajack Feb 8, 2018

amilajack Feb 8, 2018

zhaozhiming Feb 9, 2018

zhaozhiming commented Feb 9, 2018

amilajack commented Feb 10, 2018

amilajack Feb 13, 2018

zhaozhiming Feb 14, 2018

amilajack Feb 13, 2018

zhaozhiming Feb 14, 2018

zhaozhiming Feb 14, 2018

amilajack Feb 14, 2018 •

edited

zhaozhiming Feb 14, 2018

amilajack Feb 22, 2018

add prettier to format js files #1418

add prettier to format js files #1418

Conversation

zhaozhiming commented Feb 8, 2018 • edited by amilajack

amilajack commented Feb 8, 2018

amilajack commented Feb 8, 2018

zhaozhiming commented Feb 8, 2018

zhaozhiming commented Feb 8, 2018

A

B

amilajack commented Feb 8, 2018 • edited

zhaozhiming commented Feb 8, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhaozhiming commented Feb 9, 2018

amilajack commented Feb 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amilajack Feb 14, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhaozhiming commented Feb 8, 2018 •

edited by amilajack

amilajack commented Feb 8, 2018 •

edited

amilajack Feb 14, 2018 •

edited