Skip to content
This repository has been archived by the owner on Jun 28, 2019. It is now read-only.

feat: ESLint and Prettier #135

Merged
merged 29 commits into from
Mar 7, 2019
Merged

feat: ESLint and Prettier #135

merged 29 commits into from
Mar 7, 2019

Conversation

NickyMeuleman
Copy link
Contributor

wip

@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 5, 2019

Please review those @typescript rules that are disabled.

Having trouble with parserOptions.project path. When I enter ./tsconfig.json the eslint in the project works fine, but the eslint extension in the editor freaks out, resulting in an error at the top of every file.
When I enter ./packages/frontend/tsconfig.json the editor extension pick it up no problem, but the eslint in the project (called through CLI) throws not found errors

get back to json for intellisense
@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 5, 2019

Using this PR as a place to write down issues I encounter while working this.

Lots of import/no-unresolved issues (100+)
think it is related to:
import-js/eslint-plugin-import#720

future me: It wasn't, see below

Will try to update package, hope it doesn't break airbnb ruleset.
edit: added plugin:import/typescript
Seems to work, now the errors changed to Cannot find module 'typescript-eslint-parser' (undefined:undefined)eslint(import/named)
edit2: having trouble since typescript-eslint-parser is deprecated.
trying import-js/eslint-plugin-import#1283

@NickyMeuleman
Copy link
Contributor Author

imports still not working, trying to work in a .eslintrc.js file is hard. I miss my intellisense :p

Will wait until that PR gets fleshed out and merged. Then I'll try again.

Laptop gets supertoasty while reloading the project. 100% CPU/RAM/HDD usage does that, I guess.
Sleep now, it's 1 am

@NickyMeuleman
Copy link
Contributor Author

Well I tried again before it was merged anyway.
Imports now work 🎉
Turns out I made a stupid error and also included their plugin configuration in extends, whoops!

Making many commits here so I can use this to keep track of what's done.
Don't worry, I'll squash and merge when I'm done.

@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 6, 2019

Added an optional alt prop to the TalkCard for the images alt props.
If not provided it falls back to the heading prop.

@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 6, 2019

Done with frontend!
It's wonderful, WONDERFUL

Tomorrow is backend and then extracting shared stuff to the project root. Finally restore the lint-staged.

NB: only files affected by the tsconfig in frontend are affected right now, as defined by the project key in .eslintrc. This is needed for type specific linting 🎉🚀

@NickyMeuleman
Copy link
Contributor Author

Backend was easy.

Ran into trouble when trying to add ESLint to the entire workspace (to later remove duplicates from both frontend and backend)

Seems like our .js files are not being linted

typescript-eslint/typescript-eslint#109

@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 7, 2019

Tried to centralize the eslint dependencies.

Now imports are broken again (eventhough the rules are still local)
Running ESLint results in lots of Resolve error: typescript with invalid interface loaded as resolver

@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 7, 2019

Having a base config in root and others in packages could be a problem
eslint/eslint#820

reading: https://eslint.org/docs/user-guide/configuring#configuration-cascading-and-hierarchy

The way I'm seeing it working right now is that rules I placed in root (because both frontend and backend use them) are simply not being picked up.

keep extending prettier or it blows up
weird no-unused-vars behaviour in backend. still cannot read null errors when linting entire workspace
@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 7, 2019

Well, monorepos confuse me.

Did further centralization of rules (according to eslint docs)
The Typescript resolver plugin was not installed in root somehow.
That coupled with a settings key solved import problems.

no-unused-vars started firing in backend when I centralized that rule, weird, doesn't @typescript-eslint/no-unused-vars replace it?

Running yarn lint:ws still doesn't work. It hangs a while, then outputs this error:
code_2019-03-07_17-39-53

Deleted every node_modules folder and ran yarn in root. No dice, didn't help.

All in all, ready for review. This monorepo configuration is mindbending to me so I might be making some obvious error.

@NickyMeuleman NickyMeuleman marked this pull request as ready for review March 7, 2019 16:41
Copy link
Contributor

@pantharshit00 pantharshit00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use lerna for monorepo tasks.

Otherwise great work 🙌

package.json Outdated Show resolved Hide resolved
.eslintignore Outdated Show resolved Hide resolved
.eslintrc.js Outdated Show resolved Hide resolved
@NickyMeuleman
Copy link
Contributor Author

NickyMeuleman commented Mar 7, 2019

Ok.
Going to merge this now.

Only linting inside packages/frontend or packages/backend

Linting .js files acted weird in testing. Not being picked up by the CLI tool. Because the --ext option is passed for .ts and .tsx. It refused to run when you pass .js to that --ext option.
Also see typescript-eslint/typescript-eslint#109

@NickyMeuleman NickyMeuleman merged commit acea154 into master Mar 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants