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 failed on source #340

Closed
rapiz1 opened this issue Jul 28, 2020 · 6 comments · Fixed by #354
Closed

Lint failed on source #340

rapiz1 opened this issue Jul 28, 2020 · 6 comments · Fixed by #354
Assignees
Labels
enhancement New feature or request

Comments

@rapiz1
Copy link
Contributor

rapiz1 commented Jul 28, 2020

We should fix these issues before any attempt to set up a linter on Actions.

yarn run v1.22.4
warning package.json: "dependencies" has dependency "react-helmet-async" with range "^1.0.4" that collides with a dependency in "devDependencies" of the same name with version "1.0.6"
$ umi g tmp && npm run lint:js && npm run lint:style && npm run lint:prettier
npm WARN lifecycle The node binary used for scripts is /tmp/yarn--1595906147339-0.5859369794624159/node but npm is using /usr/bin/node itself. Use the `--scripts-prepend-node-path` option to include the path for the node binary npm was executed with.

> apisix-dashboard@1.1.0 lint:js /project/incubator-apisix-dashboard
> eslint --cache --ext .js,.jsx,.ts,.tsx --format=pretty ./src


  src/access.ts:1:16
  ⚠    1:16  Unexpected unnamed function.                                                                                func-names

  src/pages/Route/transform.ts:11:9
  ✖   11:9   Variable name upstream_header must match one of the following formats: camelCase, PascalCase, UPPER_CASE    @typescript-eslint/naming-convention
  ✖  150:5   Variable name upstream_path must match one of the following formats: camelCase, PascalCase, UPPER_CASE      @typescript-eslint/naming-convention
  ✖  151:5   Variable name upstream_header must match one of the following formats: camelCase, PascalCase, UPPER_CASE    @typescript-eslint/naming-convention
  ✖  152:5   Variable name upstream_protocol must match one of the following formats: camelCase, PascalCase, UPPER_CASE  @typescript-eslint/naming-convention
  ✖  153:5   Variable name upstream_id must match one of the following formats: camelCase, PascalCase, UPPER_CASE        @typescript-eslint/naming-convention

  src/pages/SSL/service.ts:9:10
  ✖    9:10  Variable name expire_start must match one of the following formats: camelCase, PascalCase, UPPER_CASE       @typescript-eslint/naming-convention
  ✖    9:24  Variable name expire_end must match one of the following formats: camelCase, PascalCase, UPPER_CASE         @typescript-eslint/naming-convention
  ✖   12:5   Use an object spread instead of Object.assign eg: { ...foo }.                                               prefer-object-spread

  src/pages/Upstream/Create.tsx:6:25
  ✖    6:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖    7:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖   33:39  Unexpected string concatenation.                                                                            prefer-template

  src/components/PluginPage/PluginPage.tsx:104:25
  ✖  104:25  Use an object spread instead of Object.assign eg: { ...foo }.                                               prefer-object-spread
  ✖  117:20  Use an object spread instead of Object.assign eg: { ...foo }.                                               prefer-object-spread

  src/pages/Consumer/Create.tsx:4:25
  ✖    4:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖    5:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates

  src/pages/Consumer/List.tsx:8:25
  ✖    8:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖    9:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates

  src/pages/Route/List.tsx:7:25
  ✖    7:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖    8:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates

  src/pages/SSL/Create.tsx:5:25
  ✖    5:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖    8:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates

  src/pages/Upstream/List.tsx:6:25
  ✖    6:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates
  ✖    7:25  /project/incubator-apisix-dashboard/node_modules/umi/index.js imported multiple times.       import/no-duplicates

  src/components/PluginForm/locales/en-US.ts:72:62
  ✖   72:62  Unexpected template string expression.                                                                      no-template-curly-in-string

  src/components/PluginForm/locales/zh-CN.ts:72:62
  ✖   72:62  Unexpected template string expression.                                                                      no-template-curly-in-string

  1 warning
  25 errors
@rapiz1

This comment has been minimized.

@juzhiyuan
Copy link
Member

Some developers may skip the lint step, could you please help to setup the lint Action?

@juzhiyuan juzhiyuan added the enhancement New feature or request label Jul 28, 2020
@rapiz1
Copy link
Contributor Author

rapiz1 commented Jul 28, 2020

@juzhiyuan I think I can open a PR to fix most of the warnings. But the big question is how to refactor the name convention like foo_bar. I will open a PR soon to do what I can do, and then leave the important issues about refactor to the core teams.

Some developers may skip the lint step, could you please help to setup the lint Action?

Once lint is set up, PR that failed the lint test couldn't be merged. So it can't be skipped in any way.

@rapiz1
Copy link
Contributor Author

rapiz1 commented Jul 28, 2020

I have fixed some lint errors. Here's what left:


  src/pages/Route/transform.ts:11:9
  ✖   11:9   Variable name upstream_header must match one of the following formats: camelCase, PascalCase, UPPER_CASE    @typescript-eslint/naming-convention
  ✖  150:5   Variable name upstream_path must match one of the following formats: camelCase, PascalCase, UPPER_CASE      @typescript-eslint/naming-convention
  ✖  151:5   Variable name upstream_header must match one of the following formats: camelCase, PascalCase, UPPER_CASE    @typescript-eslint/naming-convention
  ✖  152:5   Variable name upstream_protocol must match one of the following formats: camelCase, PascalCase, UPPER_CASE  @typescript-eslint/naming-convention
  ✖  153:5   Variable name upstream_id must match one of the following formats: camelCase, PascalCase, UPPER_CASE        @typescript-eslint/naming-convention

  src/pages/SSL/service.ts:9:10
  ✖    9:10  Variable name expire_start must match one of the following formats: camelCase, PascalCase, UPPER_CASE       @typescript-eslint/naming-convention
  ✖    9:24  Variable name expire_end must match one of the following formats: camelCase, PascalCase, UPPER_CASE         @typescript-eslint/naming-convention

  src/components/PluginForm/locales/en-US.ts:72:62
  ✖   72:62  Unexpected template string expression.                                                                      no-template-curly-in-string

  src/components/PluginForm/locales/zh-CN.ts:72:62
  ✖   72:62  Unexpected template string expression.                                                                      no-template-curly-in-string

What do you think of the naming? upstream_header is not a valid variable name according to eslint. What about upstreamHeader?

' 本地目录为 /tmp/${区域名},修改默认区域名必须同时修改 config.yaml', This line fails the lint because eslint confuses it with the format string usage. I don't know how to work around this since we're using all eslint rules from fabric. I suggest changing it to /tmp/$区域名

@juzhiyuan
Copy link
Member

Thanks, I need some time to have a conclusion here.

@juzhiyuan juzhiyuan self-assigned this Jul 28, 2020
@juzhiyuan juzhiyuan added this to TODO first in APISIX Dashboard Next Jul 28, 2020
@juzhiyuan
Copy link
Member

After some conclusion, we decide to allow snake_case variable name due to some logics from API, welcome PR to enable this feature.

APISIX Dashboard Next automation moved this from TODO first to Done Aug 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

2 participants