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

grpc-js is incredibly difficult to lint/prettify/fix #2464

Closed
dancrumb opened this issue Jun 15, 2023 · 3 comments
Closed

grpc-js is incredibly difficult to lint/prettify/fix #2464

dancrumb opened this issue Jun 15, 2023 · 3 comments

Comments

@dancrumb
Copy link
Contributor

Problem description

As a would-be contributor, I'd like to comply with the lining and formatting rules, but it's incredibly difficult to do so.

The current master HEAD does not currently pass the lint script and the fix script fails.

It's also worth noting that this project uses tslint which has been deprecated for over 4 years.

In short, it's hard to know where to start with the lining and formatting rules since the repo doesn't appear to be in compliance with its own rules.

Reproduction steps

  1. Clone repo
  2. Run npm install from the repo root
  3. cd packages/grpc-js/
  4. npm install
  5. npm run lint
    First, you get this warning:
version: 18
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <4.5.0

YOUR TYPESCRIPT VERSION: 4.9.5

Please only submit bug reports when using the officially supported version.

============= 
then you get this result:
✖ 58 problems (56 errors, 2 warnings)
  56 errors and 0 warnings potentially fixable with the `--fix` option.
  1. So, run npm run fix
    Same warning
    The command fails with ✖ 15 problems (5 errors, 10 warnings)

Environment

  • Darwin danpersallaptop.lan 21.6.0 Darwin Kernel Version 21.6.0: Mon Apr 24 21:10:53 PDT 2023; root:xnu-8020.240.18.701.5~1/RELEASE_X86_64 x86_64
  • v18.15.0
  • nvm
  • n/a
  • @grpc/grpc-js@1.8.14

Additional context

This isn't just a nitpick. I'd like to assist with making this package Deno-ready and part of this would include adding lint rules to prevent the direct use of Timer functions so that they can be properly shimmed to support Node and Deno environments.

I'd be happy to put in the work to migrate this repo from gts and tslint to eslint/prettier.
This would make life a lot easier for VSCode users, as well as bring the repo up to date with industry norms (see https://npmtrends.com/eslint-vs-gts-vs-prettier-vs-tslint).

To be clear: I'm not trying to impugn your choice with GTS... it shows/showed a lot of promise, but it seems to have stagnated or, at least, isn't a priority. They created a 4.0.1 version in January, but didn't actually publish it until March and it's still tagged as next rather than latest tag.

@dancrumb
Copy link
Contributor Author

FWIW - I tried updating GTS to version 4, but the same problems persist

@dancrumb
Copy link
Contributor Author

Here's the full error when trying to run GTS fix:

❯ npm run fix

> @grpc/grpc-js@1.8.14 fix
> gts fix src/*.ts

version: 18
=============

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

You may find that it works just fine, or you may not.

SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.1.0

YOUR TYPESCRIPT VERSION: 5.1.3

Please only submit bug reports when using the officially supported version.

=============

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/call-interface.ts
  97:27  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/client-interceptors.ts
  218:27  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/duration.ts
  34:35  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/load-balancer-outlier-detection.ts
  554:71  error  Parsing error: ')' expected

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/load-balancing-call.ts
  261:9  error  Unexpected lexical declaration in case block  no-case-declarations

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/logging.ts
  21:21  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  21:45  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  24:20  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  24:44  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  27:21  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
  27:45  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/server-call.ts
  566:11  error    Unexpected aliasing of 'this' to local variable  @typescript-eslint/no-this-alias
  626:15  warning  Unexpected any. Specify a different type         @typescript-eslint/no-explicit-any

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/server.ts
  906:4  error  Parsing error: ',' expected

/Users/danrumney/Projects/grpc-node/packages/grpc-js/src/transport.ts
  504:9  error  'call' is never reassigned. Use 'const' instead  prefer-const

✖ 15 problems (5 errors, 10 warnings)

Error: Command failed with exit code 1: node ./node_modules/eslint/bin/eslint --fix src/admin.ts src/backoff-timeout.ts src/call-credentials.ts src/call-interface.ts src/call-number.ts src/call.ts src/channel-credentials.ts src/channel-options.ts src/channel.ts src/channelz.ts src/client-interceptors.ts src/client.ts src/compression-algorithms.ts src/compression-filter.ts src/connectivity-state.ts src/constants.ts src/control-plane-status.ts src/deadline.ts src/duration.ts src/error.ts src/events.ts src/experimental.ts src/filter-stack.ts src/filter.ts src/http_proxy.ts src/index.ts src/internal-channel.ts src/load-balancer-child-handler.ts src/load-balancer-outlier-detection.ts src/load-balancer-pick-first.ts src/load-balancer-round-robin.ts src/load-balancer.ts src/load-balancing-call.ts src/logging.ts src/make-client.ts src/max-message-size-filter.ts src/metadata.ts src/object-stream.ts src/picker.ts src/resolver-dns.ts src/resolver-ip.ts src/resolver-uds.ts src/resolver.ts src/resolving-call.ts src/resolving-load-balancer.ts src/retrying-call.ts src/server-call.ts src/server-credentials.ts src/server.ts src/service-config.ts src/status-builder.ts src/stream-decoder.ts src/subchannel-address.ts src/subchannel-call.ts src/subchannel-interface.ts src/subchannel-pool.ts src/subchannel.ts src/tls-helpers.ts src/transport.ts src/uri-parser.ts
    at makeError (/Users/danrumney/Projects/grpc-node/packages/grpc-js/node_modules/gts/node_modules/execa/lib/error.js:60:11)
    at handlePromise (/Users/danrumney/Projects/grpc-node/packages/grpc-js/node_modules/gts/node_modules/execa/index.js:118:26)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async run (/Users/danrumney/Projects/grpc-node/packages/grpc-js/node_modules/gts/build/src/cli.js:120:17) {
  shortMessage: 'Command failed with exit code 1: node ./node_modules/eslint/bin/eslint --fix src/admin.ts src/backoff-timeout.ts src/call-credentials.ts src/call-interface.ts src/call-number.ts src/call.ts src/channel-credentials.ts src/channel-options.ts src/channel.ts src/channelz.ts src/client-interceptors.ts src/client.ts src/compression-algorithms.ts src/compression-filter.ts src/connectivity-state.ts src/constants.ts src/control-plane-status.ts src/deadline.ts src/duration.ts src/error.ts src/events.ts src/experimental.ts src/filter-stack.ts src/filter.ts src/http_proxy.ts src/index.ts src/internal-channel.ts src/load-balancer-child-handler.ts src/load-balancer-outlier-detection.ts src/load-balancer-pick-first.ts src/load-balancer-round-robin.ts src/load-balancer.ts src/load-balancing-call.ts src/logging.ts src/make-client.ts src/max-message-size-filter.ts src/metadata.ts src/object-stream.ts src/picker.ts src/resolver-dns.ts src/resolver-ip.ts src/resolver-uds.ts src/resolver.ts src/resolving-call.ts src/resolving-load-balancer.ts src/retrying-call.ts src/server-call.ts src/server-credentials.ts src/server.ts src/service-config.ts src/status-builder.ts src/stream-decoder.ts src/subchannel-address.ts src/subchannel-call.ts src/subchannel-interface.ts src/subchannel-pool.ts src/subchannel.ts src/tls-helpers.ts src/transport.ts src/uri-parser.ts',
  command: 'node ./node_modules/eslint/bin/eslint --fix src/admin.ts src/backoff-timeout.ts src/call-credentials.ts src/call-interface.ts src/call-number.ts src/call.ts src/channel-credentials.ts src/channel-options.ts src/channel.ts src/channelz.ts src/client-interceptors.ts src/client.ts src/compression-algorithms.ts src/compression-filter.ts src/connectivity-state.ts src/constants.ts src/control-plane-status.ts src/deadline.ts src/duration.ts src/error.ts src/events.ts src/experimental.ts src/filter-stack.ts src/filter.ts src/http_proxy.ts src/index.ts src/internal-channel.ts src/load-balancer-child-handler.ts src/load-balancer-outlier-detection.ts src/load-balancer-pick-first.ts src/load-balancer-round-robin.ts src/load-balancer.ts src/load-balancing-call.ts src/logging.ts src/make-client.ts src/max-message-size-filter.ts src/metadata.ts src/object-stream.ts src/picker.ts src/resolver-dns.ts src/resolver-ip.ts src/resolver-uds.ts src/resolver.ts src/resolving-call.ts src/resolving-load-balancer.ts src/retrying-call.ts src/server-call.ts src/server-credentials.ts src/server.ts src/service-config.ts src/status-builder.ts src/stream-decoder.ts src/subchannel-address.ts src/subchannel-call.ts src/subchannel-interface.ts src/subchannel-pool.ts src/subchannel.ts src/tls-helpers.ts src/transport.ts src/uri-parser.ts',
  escapedCommand: 'node "./node_modules/eslint/bin/eslint" --fix "src/admin.ts" "src/backoff-timeout.ts" "src/call-credentials.ts" "src/call-interface.ts" "src/call-number.ts" "src/call.ts" "src/channel-credentials.ts" "src/channel-options.ts" "src/channel.ts" "src/channelz.ts" "src/client-interceptors.ts" "src/client.ts" "src/compression-algorithms.ts" "src/compression-filter.ts" "src/connectivity-state.ts" "src/constants.ts" "src/control-plane-status.ts" "src/deadline.ts" "src/duration.ts" "src/error.ts" "src/events.ts" "src/experimental.ts" "src/filter-stack.ts" "src/filter.ts" "src/http_proxy.ts" "src/index.ts" "src/internal-channel.ts" "src/load-balancer-child-handler.ts" "src/load-balancer-outlier-detection.ts" "src/load-balancer-pick-first.ts" "src/load-balancer-round-robin.ts" "src/load-balancer.ts" "src/load-balancing-call.ts" "src/logging.ts" "src/make-client.ts" "src/max-message-size-filter.ts" "src/metadata.ts" "src/object-stream.ts" "src/picker.ts" "src/resolver-dns.ts" "src/resolver-ip.ts" "src/resolver-uds.ts" "src/resolver.ts" "src/resolving-call.ts" "src/resolving-load-balancer.ts" "src/retrying-call.ts" "src/server-call.ts" "src/server-credentials.ts" "src/server.ts" "src/service-config.ts" "src/status-builder.ts" "src/stream-decoder.ts" "src/subchannel-address.ts" "src/subchannel-call.ts" "src/subchannel-interface.ts" "src/subchannel-pool.ts" "src/subchannel.ts" "src/tls-helpers.ts" "src/transport.ts" "src/uri-parser.ts"',
  exitCode: 1,
  signal: undefined,
  signalDescription: undefined,
  stdout: undefined,
  stderr: undefined,
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
}

@murgatroid99
Copy link
Member

I would support updating or switching the linter. It looks like some actual code changes will also be needed to fix some lint errors.

dancrumb added a commit to dancrumb/grpc-node that referenced this issue Jun 15, 2023
GTS provides config for ESLint, Prettier and TSConfig; this commit removes GTS, but brings over the configuration details

Fixes grpc#2464
dancrumb added a commit to dancrumb/grpc-node that referenced this issue Jun 15, 2023
murgatroid99 added a commit that referenced this issue Jun 15, 2023
sergiitk pushed a commit to sergiitk/grpc-node that referenced this issue Jun 28, 2023
Moving from exporting a namespace to just putting assert2 functions into their own files

Fixes grpc#2464
sergiitk pushed a commit to sergiitk/grpc-node that referenced this issue Jun 28, 2023
GTS provides config for ESLint, Prettier and TSConfig; this commit removes GTS, but brings over the configuration details

Fixes grpc#2464
sergiitk pushed a commit to sergiitk/grpc-node that referenced this issue Jun 28, 2023
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

No branches or pull requests

2 participants