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

Option for readonly types #163

Closed
jonaskello opened this issue Oct 28, 2020 · 6 comments · Fixed by #691
Closed

Option for readonly types #163

jonaskello opened this issue Oct 28, 2020 · 6 comments · Fixed by #691
Labels

Comments

@jonaskello
Copy link
Collaborator

Would it be possible to add an option to generate the types with readonly?

For example instead of generating this:

export interface GetFullGraphResponse {
  nodes: GraphNode[];
  edges: GraphEdge[];
}

I would like to have this generated:

export interface GetFullGraphResponse {
  readonly nodes: readonly GraphNode[];
  readonly edges: readonly GraphEdge[];
}

The reason behind this is that our team is working in a functional style and therefore trying to keep everything immutable as far as possible. For example if we have a function to generate an array of items, that array is always readonly. So we cannot assign our function's return values directly to ts-proto generated types today .

I'm not sure it it would be possible to have readonly types in all cases but if it is feasible and in line with the goals of this project I may be able to work on a PR for it.

@stephenh
Copy link
Owner

My initial/short-term suggestion would be to try and make a DeepReadOnly that you could use in your return types against the existing ts-proto types:

microsoft/TypeScript#13923

Just as a better stop-gap than writing bespoke/hand-written types for each of your functions.

But otherwise, yeah, I think some sort of useReadOnlyTypes=true flag makes sense. Disclaimer that making the types read only will probably break some of the other generated decode/fromPartial/etc. code, which will have to be adapted to immutably create the instances. Which wouldn't be a bad thing either.

@jonaskello
Copy link
Collaborator Author

Any news on this?

@stephenh
Copy link
Owner

@jonaskello nope; would be great if you wanted to submit a PR, the code to change would be right here:

https://github.com/stephenh/ts-proto/blob/main/src/main.ts#L672

@jonaskello
Copy link
Collaborator Author

Yes, I might attempt a PR for this :-). I did some initial experimenting and changed the code you referenced to output readonly modifiers. It was quite simple, however once I did that, the code in decode and fromPartial did not compile becuase it mutates the members that are now readonly.

Simple example of generated code that does not compile with readonly modifer:

// ...

export interface SimpleMessage {
  readonly numberField: number;
}

export const SimpleMessage = {

// ...

  decode(input: Reader | Uint8Array, length?: number): SimpleMessage {
    const reader = input instanceof Reader ? input : new Reader(input);
    let end = length === undefined ? reader.len : reader.pos + length;
    const message = createBaseSimpleMessage();
    while (reader.pos < end) {
      const tag = reader.uint32();
      switch (tag >>> 3) {
        case 1:
          message.numberField = reader.int32(); // This line does not compile
          break;
        default:
          reader.skipType(tag & 7);
          break;
      }
    }
    return message;
  },

  //...

  fromPartial<I extends Exact<DeepPartial<SimpleMessage>, I>>(object: I): SimpleMessage {
    const message = createBaseSimpleMessage();
    message.numberField = object.numberField ?? 0; // This line does not compile
    return message;
  },
};

// ...

@stephenh Do you have a suggestion how to work around this?

@stephenh
Copy link
Owner

@jonaskello ah sure... FWIW I'd probably just thrown in an as any like:

    const message = createBaseSimpleMessage() as any;

If we're in "readonlyTypes" mode. And personally since we're in "trusted" / generated code, I'd be fine with that "as any" white-lie to the type system.

That said, looking around, it also looks like we could add a utility Writeable like this:

type Writeable<T> = { -readonly [P in keyof T]: T[P] };

Which would be conditionally output, similar to how our DeepPartial utility type is sometimes included/sometimes now, and then cast the message like:

    const message = createBaseSimpleMessage() as Writeable<SimpleMessage>;

...or maybe have createBaseSimpleMessage() itself just return a Writeable<SimpleMessage>?

That would be more kosher than the "as any".

Fwiw if you're adding a readonly param, we could also do an Object.freeze(message) to not only enforce readonly in the type system, but the runtime as well?

@stephenh
Copy link
Owner

🎉 This issue has been resolved in version 1.130.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

zfy0701 added a commit to sentioxyz/ts-proto that referenced this issue Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants