Introduce beautify-lint and reformat files automatically

[okonet]

What kind of change does this PR introduce?
refactoring + build related change

Did you add tests for your changes?
N/A

If relevant, did you update the documentation?
N/A

Summary

To ensure code style is consistent with the main webpack repo, I've added the pretest command that will lint it on CI. Also I've added an additional step to lint-staged that will automatically reformat code according to code style rules in a pre-commit hook.

Does this PR introduce a breaking change?
No

[evenstensberg]

Mind fixing conflicts first?

[okonet]

@ev1stensberg lol you're too fast :)

[okonet]

Unfortunately, I've encountered some inconsistencies between beautify-rewrite and eslint config. Even after I've updated eslint config to the webpack's one to reduce code style inconsistencies I still see some cases where beautify-rewrite formats code differently as eslint --fix does.

Try running npm run lint:style to see those inconsistencies.

cc @sokra

[evenstensberg]

What is needed to fix the issue @okonet ?

[okonet]

@ev1stensberg I guess the issue is with how beautify formats the code in some cases since I'm using same ESLint and jsbeautify configs as in webpack's repo.

[evenstensberg]

If the eslint and beautify code is the same, shouldn't it be no problem at all? @okonet

[evenstensberg]

Status on this?

[pksjce]

This would really help us to rebase with the main webpack repo's cli. @okonet - Can I help you with this?

[okonet]

I'm not sure how I can help. I'd propose we switch both repos to prettier since it shouldn't have those issues AFAIK.

[okonet]

webpack/webpack#4757

[DanielaValero]

Hi @okonet @pksjce so, we proceed on this by basing our settings on webpack's one? If so, I'd take over the PR to make the required updates

[DanielaValero]

Hello @okonet @pksjce

I have updated this PR with the next:

• Updated the outdated dependencies
• Replaced js-beaufity with prettier
• Reformat the files with the lintin settings

Would you check if the changes are ok at your end?

[okonet]

I don't think this is needed if you already have jest as a dependency.

[DanielaValero]

Actually I added it because I ran the tests with npm test and node complained that jest-cli is not in the project

[okonet]

Any reason for not including it? I believe it's much more readable with spaces.

[DanielaValero]

let me double check, I was getting crazy errors of compatibility between eslint and prettify, did some tests, this might have squeezed in.

[okonet]

Looking at all conflicts I think it would be easier to start over...

[evenstensberg]

+1 @okonet

[DanielaValero]

Totally! Will create a new branch and cherry-pick the changes. Afterwards will add a single commit with the style fixes. Will close this PR and open a new one

[DanielaValero]

Hi @okonet @pksjce,

I've been playing around and trying to get the automated code beatutifying work with prettier but has been quite complicated.

Issues I've got:

• Cyclical issues with eslint: Sometimes eslint complains about something, I fix it and prettier complains
• The error messages that prettier outputs are not really clear, therefore, I've invested much time on figuring out what the issue is. As the nature of this project is more like: work-on-our-free-time, I'd rather invest that time on fixing bugs than figuring out what the lintin issue is
• The integration with eslint seems quite blurry at the moment, running eslint --fix together with prettier, in some cases has even introduced in the code syntax errors.

At the moment, my conclusion is:

• Prettier is good for what we are using it, to be run in the code the cli creates
• Replacing js-beautify by prettier is something we could try again in the future, once the integration with eslint is stronger, and prettier has more development time.

What you guys think?

[okonet]

@DanielaValero I'm running prettier with eslint on all my code and never run into issue. I assume you're running into conflicting rules that needs to be disabled. See https://github.com/okonet/eslint-config-okonet/blob/master/index.js for my setup. The important thing is https://github.com/prettier/eslint-config-prettier that disables all styling rules of ESLint.

[DanielaValero]

HI @okonet thanks for the pointer, will try it out. Because #173 touches many files, I could try it out in a separate branch after that one is merged, otherwise will be nightmare to fix merge conflicts once more prs get merged