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

Incorrect message names in the generated code for repeated fields #1072

Closed
avlyalin opened this issue Jul 10, 2024 · 2 comments · Fixed by #1073 · May be fixed by habyyb/CD-AdvancedDigitalIQ#3
Closed

Incorrect message names in the generated code for repeated fields #1072

avlyalin opened this issue Jul 10, 2024 · 2 comments · Fixed by #1073 · May be fixed by habyyb/CD-AdvancedDigitalIQ#3
Labels

Comments

@avlyalin
Copy link
Contributor

avlyalin commented Jul 10, 2024

Hi! Thanks for the lib 🙂

Problem

In the proto files for which I generate TS code, there are messages with the same names but in different packages. For example, within one message, there can be a field of a type with the same name, but from a different package.
It's easier to demonstrate with an example:

// model.proto
syntax = "proto3";

package model;

message User {
    string id = 1;
}
// api.proto
syntax = "proto3";

package api;

import "model.proto";

message User {
    repeated model.User users = 1;
    model.User user = 2;
}

In the code generated for api.proto, the import of User includes an alias to avoid collisions:

import { User as User1 } from "./model.js";

However, in the constructor methods toJson and fromJson of User (in api.ts) incorrect message name is generated. Instead of the alias User1, there is User (User.fromJson/User.toJson) in map-functions:

// api.ts

export const User = {
...

  fromJSON(object: any): User {
    return {
      // expected: ... => User1.fromJSON(e)) : [],
      users: globalThis.Array.isArray(object?.users) ? object.users.map((e: any) => User.fromJSON(e)) : [],
      user: isSet(object.user) ? User1.fromJSON(object.user) : undefined,
    };
  },

  toJSON(message: User): unknown {
    const obj: any = {};
    if (message.users?.length) {
      // expected: ... => User1.toJSON(e));
      obj.users = message.users.map((e) => User.toJSON(e));
    }
    if (message.user !== undefined) {
      obj.user = User1.toJSON(message.user);
    }
    return obj;
  },

...
}

For non-repeated fields, the correct alias User1 is created.

Solution

After studying the code, I guess I found a place with an error. Calling toCodeString, toString leads to caching of the calculated value (code, codeWithImports).
As far as I understand, because of this, when code is generated from all the collected Code instances later, these objects retain an incorrect value that does not take into account the necessary parameters/context.

I've opened a small PR, that fixes this.

@stephenh
Copy link
Owner

Ah! Your diagnosis is exactly right; we need to keep the readSnippet as the Code object for its import to get rewritten appropriately. Thanks for the fix and great PR!

stephenh pushed a commit that referenced this issue Jul 13, 2024
## [1.181.1](v1.181.0...v1.181.1) (2024-07-13)

### Bug Fixes

* Incorrect message names in the generated code for repeated fields ([#1073](#1073)) ([8a95d8e](8a95d8e)), closes [#1072](#1072)
@stephenh
Copy link
Owner

🎉 This issue has been resolved in version 1.181.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants