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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint: ignore js file #4431

Merged
merged 1 commit into from
Jan 15, 2020
Merged

lint: ignore js file #4431

merged 1 commit into from
Jan 15, 2020

Conversation

jannyHou
Copy link
Contributor

@jannyHou jannyHou commented Jan 14, 2020

Fixes #4205

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 👈

@@ -12,6 +12,7 @@ if (require.main === module) {
const config = {
rest: {
port: +process.env.PORT || 3000,
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ts eslint rules like @typescript-eslint/prefer-nullish-coalescing shouldn't check js file

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it required for the build to pass? We already have this override - https://github.com/strongloop/loopback-next/blob/master/packages/eslint-config/eslintrc.js#L158

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raymondfeng Not really, our code base is good, and adding this line results in lint error...
But the todo example created by lb4 example complains:

lb4 example (choose todo)
cd loopback4-example-todo
npm i
npm run lint

> @loopback/example-todo@1.9.4 lint /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> npm run prettier:check && npm run eslint
> @loopback/example-todo@1.9.4 prettier:check /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> npm run prettier:cli -- -l
> @loopback/example-todo@1.9.4 prettier:cli /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-prettier "**/*.ts" "-l"
> @loopback/example-todo@1.9.4 eslint /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-eslint --report-unused-disable-directives .
/Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo/index.js
  15:30  error  Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator  @typescript-eslint/prefer-nullish-coalescing

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I wonder why https://github.com/strongloop/loopback-next/blob/master/packages/eslint-config/eslintrc.js#L158 does not exclude js files. Can you check which version of @loopback/eslint-config is pulled in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raymondfeng It's already on the latest released version @loopback/eslint-config@5.0.1

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raymondfeng 👍 tested the config file, I think the necessary config is extends: ['@loopback/eslint-config']

Details verified in comment #4431 (comment)

@jannyHou
Copy link
Contributor Author

Verified with the latest cloned todo example:

before adding the .eslintrc.js file
jannyHous-MacBook-Pro:loopback4-example-todo jannyhou$ npm run lint

> @loopback/example-todo@1.9.4 lint /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> npm run prettier:check && npm run eslint


> @loopback/example-todo@1.9.4 prettier:check /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> npm run prettier:cli -- -l


> @loopback/example-todo@1.9.4 prettier:cli /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-prettier "**/*.ts" "-l"


> @loopback/example-todo@1.9.4 eslint /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-eslint --report-unused-disable-directives .


/Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo/index.js
  15:30  error  Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator  @typescript-eslint/prefer-nullish-coalescing

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @loopback/example-todo@1.9.4 eslint: `lb-eslint --report-unused-disable-directives .`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @loopback/example-todo@1.9.4 eslint script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/jannyhou/.strong-registry/official.cache/_logs/2020-01-14T21_08_58_677Z-debug.log
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @loopback/example-todo@1.9.4 lint: `npm run prettier:check && npm run eslint`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the @loopback/example-todo@1.9.4 lint script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/jannyhou/.strong-registry/official.cache/_logs/2020-01-14T21_08_58_705Z-debug.log

jannyHous-MacBook-Pro:loopback4-example-todo jannyhou$ npm run clean

> @loopback/example-todo@1.9.4 clean /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-clean *example-todo*.tgz dist tsconfig.build.tsbuildinfo package

after adding the .eslintrc.js file
jannyHous-MacBook-Pro:loopback4-example-todo jannyhou$ npm run lint

> @loopback/example-todo@1.9.4 lint /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> npm run prettier:check && npm run eslint


> @loopback/example-todo@1.9.4 prettier:check /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> npm run prettier:cli -- -l


> @loopback/example-todo@1.9.4 prettier:cli /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-prettier "**/*.ts" "-l"


> @loopback/example-todo@1.9.4 eslint /Users/jannyhou/Desktop/2019/ui/test/loopback4-example-todo
> lb-eslint --report-unused-disable-directives .

@jannyHou jannyHou self-assigned this Jan 14, 2020
@jannyHou jannyHou merged commit 51370bc into master Jan 15, 2020
@jannyHou jannyHou deleted the lint/todo-example branch January 15, 2020 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

lb4 repository,model,controller commands generate artifacts with 'lint' issues
2 participants