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: virtual oneof property #1099

Open
paskozdilar opened this issue Aug 27, 2024 · 2 comments
Open

Feature request: virtual oneof property #1099

paskozdilar opened this issue Aug 27, 2024 · 2 comments

Comments

@paskozdilar
Copy link

The protobuf.js library provides an option oneofs for setting virtual oneof fields that contain name of the set property:

    /** Includes virtual oneof properties set to the present field's name, if any */
    oneofs?: boolean;

Similar option already exists in the generated pbjs.d.ts files in integration tests, e.g. in oneof-properties:

export namespace oneof {
    // ...
    class PleaseChoose implements IPleaseChoose {
        // ...
        public choice?: ("aNumber"|"aString"|"aMessage"|"aBool"|"bunchaBytes"|"anEnum");
        public eitherOr?: ("either"|"or"|"thirdOption");
        // ...
    }
    // ...
}

It would be ideal to have generated interface actually be a flat union type with a virtual parameter, so TypeScript can check for field existence based on virtual field value, but my naive attempt has caused combinatorial explosion of union types related to Exact and DeepPartial which are too complicated for me to tackle at the moment.

Having just a virtual parameter is a weaker - but still useful - feature.

I am planning to do the work once I get more comfortable with the codebase. Any pointers would be welcome :)

@stephenh
Copy link
Owner

Hi @paskozdilar ! Just sanity checking, but have you seen our existing oneof handling, i.e.:

https://github.com/stephenh/ts-proto?tab=readme-ov-file#oneof-handling

And specifically the oneof=unions-value option?

It outputs code like:

interface YourMessage {
  eitherField?: { $case: "field_a"; value: string } | { $case: "field_b"; value: string };
}

Which seems very similar to this request, but I can't 100% tell ... is there something about the unions-value output (or older unions output) that is different from your proposal/what you'd like to see?

@paskozdilar
Copy link
Author

paskozdilar commented Sep 13, 2024

Yes, it's very similar, but unfortunately the oneof=unions-value is not compatible with default grpc-gateway setup, which uses flat representation like ts-proto default.

However, the problem with the default flat-representation of ts-proto is twofold:

  1. It's impossible to know from ts-proto generated types alone which fields belong to which oneof
  2. It's impossible to assert that we've checked for all types available in a oneof

The problems could be solved by the virtual property field that would contain the name of the set oneof property, e.g.:

message Foo {
  oneof either_field { string field_a = 1; string field_b = 2; }
}

would generate the following:

export interface Foo {
  eitherField: 'fieldA' | 'fieldB' | undefined;
  fieldA: string | undefined;
  fieldB: string | undefined;
}

// we can now do this:
function fn(foo: Foo) {
    switch (foo.eitherField) {
        case 'fieldA':
            const fieldA = foo[foo.eitherField] as typeof Foo[foo.eitherField];
            // ... handle field a
        case 'field_b':
            const fieldB = foo[foo.eitherField] as typeof Foo[foo.eitherField];
            // ... handle field b
        default:
            // compiler will fail here if we haven't exhausted all non-undefined cases
            let _: undefined;
            _ = args.eitherField;
    }
}

That wouldn't give complete type-safety, but would still increase convenience.

In the current implementation of flat-fields, all oneofs have to be checked manually, and one has to look at the proto file itself to know which field belongs to which oneof.


Additionally, if we had a true union as message type, we could avoid the typecasting (... as typeof Foo[args.either_field]) we could use a mixture of union-types and flat represenatation:

export type Foo = {
  eitherField: 'fieldA';
  fieldA: string;
} | {
  eitherField: 'fieldB';
  fieldB: string;
} | {
  eitherField: undefined;
}

// we can now do this:
function fn(foo: Foo) {
    switch (foo.eitherField) {
        case 'fieldA':
            const fieldA = foo.fieldA; // typescript infers string automatically
            const _ = foo.fieldB; // error: property "fieldB" doesn't exist on { eitherField: "fieldA", fieldA: string }
            // ... handle field a
        case 'fieldB':
            const fieldB = foo.fieldB;
            // ... handle field b
        default:
            // compiler will fail here if we haven't exhausted all non-undefined cases
            let _: undefined;
            _ = args.eitherField;
    }
}

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

No branches or pull requests

2 participants