Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

WIP: Eslint / Prettier integration. #179

Closed
LaRuaNa wants to merge 10 commits into from
Closed

WIP: Eslint / Prettier integration. #179

LaRuaNa wants to merge 10 commits into from

Conversation

@LaRuaNa
Copy link
Contributor

@LaRuaNa LaRuaNa commented Mar 8, 2019

Hi all,

as decided in the last meeting here a experimental PR with eslint and prettier integration based on the industry standard (airbnb config). I got from couple of mates from Microsoft the info that there is a roadmap for Typescript in which they announced that "editor team will be focusing on leveraging ESLint". I think it makes sense to move official supported platforms rather than community driven ones. ( You know what I mean, don't get me wrong :] ) So I removed tslint from our config and introduced eslint. IMO It makes also sense because of all the eslint plugins like react-hooks. They're just up to date.

So, please take a look in that, try to fix couple of eslint errors and play a bit around to get a feel for the config.

Here my vscode config you might need :]

  "eslint.enable": true,
  "typescript.validate.enable": true,
  "editor.formatOnSave": true,
  "eslint.autoFixOnSave": true,
  "eslint.validate": [
    "javascript",
    "javascriptreact",
    { "language": "typescript", "autoFix": true },
    { "language": "typescriptreact", "autoFix": true }
  ],
  "prettier.eslintIntegration": true

Note: Please don't open any PRs with that config and do not merge this PR yet

refs:
microsoft/TypeScript#29288
https://eslint.org/blog/2019/01/future-typescript-eslint

@sagirk
Copy link
Contributor

sagirk commented Mar 8, 2019

MOAR! 😅 refs:
Official announcement from Palantir
TypeScript ESLint Repo
Roadmap for TypeScript ESLint

@sagirk sagirk mentioned this pull request Mar 8, 2019
Merged
@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Mar 9, 2019

Oh cool didn't know that's so far already, thanks @sagirk :]

So TLDR: "The TypeScript community intends to meet JavaScript developers where they are, and ESLint is the tool of choice for JavaScript linting. In order to avoid bifurcating the linting tool space for TypeScript, we therefore plan to deprecate TSLint and focus our efforts instead on improving ESLint’s TypeScript support"

@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Mar 11, 2019

Seems like we like that config. I'd start with the required changes so we can land this soon :]

@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Apr 3, 2019 •
edited
Loading

Quick update on that: I'm having some conflicts between eslint and prettier plus having all the time conflicts with master which are quite tedious. So hope in couple days it'll be done :]

@ahmadawais
Copy link
Member

ahmadawais commented Apr 18, 2019

I really would like this to be merged soon ;) Anything from your end at @LaRuaNa 🤔

@amiller-gh amiller-gh added the wr-agenda Website Redesign Meeting Agenda Label label May 9, 2019
@mhdawson mhdawson mentioned this pull request May 9, 2019
Closed
@mhdawson mhdawson mentioned this pull request May 20, 2019
Closed
@mhdawson mhdawson mentioned this pull request Jun 3, 2019
Closed
@Trott
Copy link
Member

Trott commented Jun 5, 2019

Rebase to remove conflicts?

@LaRuaNa LaRuaNa removed the wr-agenda Website Redesign Meeting Agenda Label label Jun 6, 2019
@MylesBorins
Copy link
Contributor

MylesBorins commented Aug 6, 2019

ping @LaRuaNa

@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Aug 6, 2019

ping @LaRuaNa
@MylesBorins I've on my to-do list. Will get back to this when I can get around :]

@LaRuaNa LaRuaNa changed the title chore: Eslint / Prettier integration. WIP: Eslint / Prettier integration. Aug 23, 2019
@jonchurch
Copy link
Contributor

jonchurch commented Sep 7, 2019

@LaRuaNa I'm poking around this PR right now, great work!

Is there anything I can help you with?

@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Sep 7, 2019

@jonchurch It's almost done. Needs to be rebased and perhaps might require some small fixes after rebase. If you feel like it take a look :]

@jonchurch
Copy link
Contributor

jonchurch commented Sep 7, 2019 •
edited
Loading

Awesome! Since we are using jest for testing, I'd suggest adding the eslint-plugin-jest and extending our config with plugin:jest/recommended.

This takes care of linting errors about global jest variables not being defined like describe and it, and also adds some "best practices" for tests (the extent of which I'm not familiar with).

I tried it out locally with these items added and it looks good.

@jonchurch jonchurch force-pushed the linter-prettier-integration branch from 2af1f94 to c69d6bb Compare September 9, 2019 03:15
@jonchurch
Copy link
Contributor

jonchurch commented Sep 9, 2019 •
edited
Loading

I rebased this branch onto master, added eslint-plugin-jest, and started an eslintrc overrides entry to disable some typescript rules for .js files.

There's likely a way to only run the typescript rules on typescript files, but I just disabled the one rule that I thought made sense. We aren't using babel for .js files so the rule preventing the use of require isn't useful for .js files.

I also changed some of the scripts, replacing the format step in scripts that used it with a lint script, since eslint is also responsible for running prettier now.

@jonchurch jonchurch requested a review from amiller-gh September 9, 2019 03:46
@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Sep 9, 2019

@jonchurch All good :] I'm also on it and somehow I'm not really happy with the config. There are some conflicts between eslint and prettier. _ Went through all the files and tried to sort them out but doesn't seem reasonable. It would be great to have eslint's autoFix and prettier at the same time. There is also a way to do that (Link) but as mentioned conflicts between all the configs we extend from make it not easy :] So ideas&help welcome.

@jonchurch
Copy link
Contributor

jonchurch commented Sep 9, 2019

@LaRuaNa I agree, this isn't ready yet. The config isn't quite there, and I didn't even realize we weren't running the prettier plugin for autofixing.

I'll keep working on this also as I'm interested in learning more about using eslint + typescript.

@jonchurch jonchurch removed the request for review from amiller-gh September 9, 2019 19:41
@jonchurch jonchurch mentioned this pull request Sep 14, 2019
Merged
@LaRuaNa
Copy link
Contributor Author

LaRuaNa commented Mar 19, 2020

Done in #315

@LaRuaNa LaRuaNa closed this Mar 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@LaRuaNa @sagirk @ahmadawais @Trott @MylesBorins @jonchurch @amiller-gh