search

grpc-ecosystem / protoc-gen-grpc-gateway-ts

protoc-gen-grpc-gateway-ts is a Typescript client generator for the grpc-gateway project. It generates idiomatic Typescript clients that connect the web frontend and golang backend fronted by grpc-gateway.
Apache License 2.0
142 stars 51 forks source link

Use `import type` for type imports in template #16

Closed remko closed 3 years ago

remko commented 3 years ago

When a proto file has imports, the generator generates code like

import * as GoogleProtobufTimestamp from "./google/protobuf/timestamp.pb";

The default setup of TypeScript in the bundler I use is strict, and is set up to give errors on type-only imports:

error TS1371: This import is never used as a value and must use 'import type' because 'importsNotUsedAsValues' is set to 'error'.

I can work around this by loosening the tsconfig setting.

Would it be possible to replace the imports generated for these kinds of import by import type to avoid these TypeScript errors on strict setups?

lyonlai commented 3 years ago

tsconfig.js is different in every project. protoc-gen-grpc-gateway-ts can't fit all of them. It would make sense to loose the constrains for the generated code. @remko

atreya2011 commented 3 years ago

Is adding the following to the beginning of the generated TypeScript files a bad idea?

/* eslint-disable */
/* tslint:disable */
lyonlai commented 3 years ago

It is bad idea to change generated code. those configuration should have been done without touching the generated code. you will need to find your own way in your project to do so.

remko commented 3 years ago

tsconfig.js is different in every project. protoc-gen-grpc-gateway-ts can't fit all of them.

'Fitting' in this case means generating strictly TypeScript-safe code, which should always be possible. Whether it is useful to do so for generated code is of course debatable. However, if you decide it is not useful for your generated code to be strictly TypeScript-safe (which sounds perfectly reasonable to me), then it makes sense to also include the tslint:disable and eslint-disable lines at the top of the generated files, as @atreya2011 suggests?

remko commented 3 years ago

FWIW, grpcweb adds the following to the top of their generated files:

/* eslint-disable */
// @ts-nocheck
lyonlai commented 3 years ago

misunderstood the comment, thought it's added after the code's generated. now I understand.

those two lines make sense then. I'll take this issue and make it either this week or next.