Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DO NOT MERGE] rework eslint config to use proper TS projects #5590

Closed
wants to merge 1 commit into from

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented May 28, 2020

Disable createDefaultProgram option and ensure every linted file belongs to some tsconfig, see https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/MONOREPO.md#getting-started---monorepo-configuration

Unfortunately, eslint is sometimes running out of memory with this new setup. I think we have to wait until typescript-eslint/typescript-eslint#1192 and/or typescript-eslint/typescript-eslint#2094 is resolved by typescript-eslint.

The pull request contains a single "all-in-one" commit right now, we will need to split it into multiple smaller commits before landing. I think the change in eslint-config is breaking backwards compatibility and needs to be described as such.

Migration guide for LB users:

Make sure all .ts and .js files in your repository are either included by tsconfig.json or excluded via .eslintignore file.

Checklist

馃憠 Read and sign the CLA (Contributor License Agreement) 馃憟

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

馃憠 Check out how to submit a PR 馃憟

Disable `createDefaultProgram` option and ensure every linted
file belongs to some `tsconfig`.

Signed-off-by: Miroslav Bajto拧 <mbajtoss@gmail.com>
@bajtos bajtos added the Internal Tooling Issues related to our tooling and monorepo infrastructore label May 28, 2020
@bajtos bajtos self-assigned this May 28, 2020
"packages/repository/examples/**/*.ts",
"packages/tsdocs/bin/*.js",
"packages/tsdocs/fixtures/**/*.js"
],
Copy link
Member Author

Choose a reason for hiding this comment

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

Discussion point: instead of adding all of those files to the dummy tsconfig for eslint only, should we add them to "real" typescript projects and run them through the compiler to get basic type checks?

@@ -5,3 +5,4 @@ coverage/
**/*.d.ts
/docs/_preview
/docs/_loopback.io
**/.eslintrc.js
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bad idea. We are running Prettier as part of eslint, this line disables prettier formatting checks for .eslintrc files.

@bajtos
Copy link
Member Author

bajtos commented Jul 28, 2020

I think #5983 is good enough for now, we can revisit the status once typescript-eslint/typescript-eslint#1192 and/or typescript-eslint/typescript-eslint#2094 is resolved.

@bajtos bajtos closed this Jul 28, 2020
@bajtos bajtos deleted the fix/eslint-config branch July 28, 2020 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked breaking-change Internal Tooling Issues related to our tooling and monorepo infrastructore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant