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

[1.1.0] Namespace missing for an inner message #837

Closed
travikk opened this issue Jun 2, 2020 · 6 comments · Fixed by #844
Closed

[1.1.0] Namespace missing for an inner message #837

travikk opened this issue Jun 2, 2020 · 6 comments · Fixed by #844

Comments

@travikk
Copy link
Contributor

travikk commented Jun 2, 2020

I noticed that with the 1.1.0 version, below proto is generated incorrectly:

message MessageOuter {
  message MessageInner {
  }

  repeated MessageInner someProp = 1;
}

Generated d.ts is resulting in:

export class MessageOuter extends ... {
  getSomeProp(): Array<MessageInner>;  // #1
  setSomeProp(value: Array<MessageInner>): MessageOuter // #2
  ...
}

export namespace MessageOuter {
  ...
  export class MessageInner extends ... {
    ...
  }
  ...
}

that will not compile, as the MessageInner should be prefixed with the namespace at #1 and #2. 1.0.7 used to generate this code just fine.

@jvmlet
Copy link

jvmlet commented Jun 3, 2020

Suffering form the same issue, show stopper for us

@stanley-cheung
Copy link
Collaborator

stanley-cheung commented Jun 4, 2020

Is this what you are referring to? The diff is backward: minuses are 1.1.0, pluses are 1.0.7.

diff --git a/test_pb.d.ts b/test_pb.d.ts
index e40dcab..553b179 100644
--- a/test_pb.d.ts
+++ b/test_pb.d.ts
@@ -1,10 +1,10 @@
 import * as jspb from "google-protobuf"
 
 export class MessageOuter extends jspb.Message {
-  getSomepropList(): Array<MessageInner>;
-  setSomepropList(value: Array<MessageInner>): MessageOuter;
-  clearSomepropList(): MessageOuter;
-  addSomeprop(value?: MessageInner, index?: number): MessageInner;
+  getSomepropList(): Array<MessageOuter.MessageInner>;
+  setSomepropList(value: Array<MessageOuter.MessageInner>): void;
+  clearSomepropList(): void;
+  addSomeprop(value?: MessageOuter.MessageInner, index?: number): MessageOuter.MessageInner;
 
   serializeBinary(): Uint8Array;
   toObject(includeInstance?: boolean): MessageOuter.AsObject;
@@ -16,7 +16,7 @@ export class MessageOuter extends jspb.Message {
 
 export namespace MessageOuter {
   export type AsObject = {
-    somepropList: Array<MessageInner.AsObject>,
+    somepropList: Array<MessageOuter.MessageInner.AsObject>,
   }
 
   export class MessageInner extends jspb.Message {

This is really strange. Our code doesn't seem to have changed. It seems that protobuf is returning different things. Still looking into this. Read the code wrong.

@travikk
Copy link
Contributor Author

travikk commented Jun 4, 2020

@stanley-cheung yeah that looks like the issue indeed.

@stanley-cheung
Copy link
Collaborator

Looks like this is due to #780. A fix for another purpose might have broken this. So perhaps a re-factor somewhere needs to be done.

@jvmlet
Copy link

jvmlet commented Jun 6, 2020

Thanks for the quick fix. When do you plan to release the version with this patch?

@stanley-cheung
Copy link
Collaborator

I have a couple of fixes I'd like to do and get them in together, so hopefully next week.

borissmidt pushed a commit to borissmidt/grpc-web that referenced this issue Feb 16, 2022
* First set of dependency updates

* Further updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants