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

[Feature Request]: proto-loader-gen-types: More options for generated code #2907

Open
RainmanVi opened this issue Feb 26, 2025 · 4 comments · May be fixed by #2912
Open

[Feature Request]: proto-loader-gen-types: More options for generated code #2907

RainmanVi opened this issue Feb 26, 2025 · 4 comments · May be fixed by #2912

Comments

@RainmanVi
Copy link

Is your feature request related to a problem? Please describe.

After extensive trial and error, I found that proto-loader-gen-types is the simplest solution for generating type definitions for code that uses @grpc/grpc-js in TypeScript.

But the TypeScript code generated by proto-loader-gen-types has the following issues:

  1. The file extension is .ts, but since it is meant for type declarations, it is supposed to be .d.ts instead.
  2. With module": "nodenext" set in tsconfig.json and "type": "module" set in package.json for Node projects, VS Code raises warnings about missing file extensions in import module specifiers. When using ESM in Node, both Node.js and TypeScript's module resolution require explicit file extensions in import paths. However, the code generated by proto-loader-gen-types does not automatically include them. This results in a manual update step after code generation, which is inconvenient and an antipattern.

Describe the solution you'd like

  1. Provide an option to configure which file extension (.ts or .d.ts) is used for generated code. Maybe something like this
  2. Provide an option to configure whether file extensions are appended to module specifiers in generated code. Maybe something like this

Additional context

There are related discussions, such as #2693 and #2401. This issue aims to revive the topic and encourage further discussion.

@murgatroid99
Copy link
Member

When I initially created that tool, I had it generate .d.ts but it caused some problems, so I switched to generating .ts. Unfortunately, I don't remember the details of those problems. I'm willing to reevaluate adding that option. I'd like to understand the impact of this issue better, so can you explain what the impact is in your usage of having a .ts file extension instead of .d.ts?

As you can see in #2693, we tried adding the file extension and it broke in some cases. I am OK with adding an option, but first I would really like to understand whether there is an extension that we can always set, and if not, why not.

@RainmanVi
Copy link
Author

@murgatroid99

  1. Using .ts or .d.ts file extensions has no inherent negative effect if they only contain type definitions, as these files are ultimately removed at runtime. The concern is purely a matter of convention and maintaining consistency with the codebase's style.

  2. There is currently no universal solution for setting the file extension in module specifiers once and for all. Module resolution is complex, and the file extension in the transpiled .js file may or may not be changed by tsc, depending on the configuration in tsconfig.json. This uncertainty makes it difficult to determine the final file extension in advance. In my own opinion, the best approach is to leave the choice to the end user. By default, the file extension is omitted, but an option is provided to explicitly include a specified extension (e.g., .js, .cjs, .mjs, .ts, .mts) in the generated code.

@murgatroid99
Copy link
Member

That seems reasonable. I would be OK with adding those options.

@tenkirin
Copy link

tenkirin commented Mar 1, 2025

@murgatroid99

Hi, I encountered the same issue and would like to submit a PR to fix it. Here are my ideas:

Add the following two options to proto-loader-gen-types:

  • targetFileExtension: File extension for generated files. Defaults to .ts
  • importFileExtension: File extension for import specifiers in generated code. Defaults to none (omitted)

The default behavior of proto-loader-gen-types is kept unchanged to avoid any possible breaking on existing usage, and the output is only modified when options are provided as needed.

Any suggestions?

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 a pull request may close this issue.

3 participants