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

Remove the rewrite_imports preamble #343

Closed
mfridman opened this issue Dec 14, 2022 · 0 comments · Fixed by #386
Closed

Remove the rewrite_imports preamble #343

mfridman opened this issue Dec 14, 2022 · 0 comments · Fixed by #386

Comments

@mfridman
Copy link
Member

I think we should consider removing the rewrite_imports preamble from the generated code. It's more of an implementation detail (and feature) of the BSR. By removing this we get:

  1. cleaner generated code.
  2. ability to improve the underlying glob patterns upstream without affecting the imports. We likely don't want to do this, but if we do then it'll be seamless to the user without diff changes.
  3. this is helpful for debugging, but an issue with this chunk of code will still result in the wrong import path. So that's the thing the end-user should be focusing on, and less on this option.
// @generated by protoc-gen-connect-web v0.3.3 with parameter "rewrite_imports=./pet/v1/**/*_pb.js:@buf/acme_petapis.bufbuild_es,rewrite_imports=./payment/v1alpha1/**/*_pb.js:@buf/acme_paymentapis.bufbuild_es,rewrite_imports=./payment/v1alpha1/**/*_connectweb.js:@buf/acme_paymentapis.bufbuild_connect-web,rewrite_imports=./google/type/**/*_pb.js:@buf/googleapis_googleapis.bufbuild_es,rewrite_imports=./google/type/**/*_connectweb.js:@buf/googleapis_googleapis.bufbuild_connect-web"
// @generated from file pet/v1/pet.proto (package pet.v1, syntax proto3)
/* eslint-disable */
// @ts-nocheck
smaye81 added a commit that referenced this issue Feb 14, 2023
Fixes #343.

This strips the `rewrite_imports` parameter from the generated file
preamble as that is mostly an implementation detail and can be very
noisy (and not very helpful) to consumers/readers of the generated code.
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.

1 participant