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

Accessing oneof dynamically #129

Closed
fenos opened this issue Jul 10, 2021 · 10 comments
Closed

Accessing oneof dynamically #129

fenos opened this issue Jul 10, 2021 · 10 comments

Comments

@fenos
Copy link
Contributor

fenos commented Jul 10, 2021

Hi,

I can't find a way nice way to access a oneof property type dynamically, maybe you can advise

Imagine I have the following one of type:

export interface Task_Payload {

    payload: {
        oneofKind: "renameTask";
        renameTask: RenameTask;
    } | {
        oneofKind: "noop";
        noop: NoopTask;
    } | {
        oneofKind: "trimming";
        trimming: TrimmingTask;
    } | {
        oneofKind: undefined;
    };
}

I want to be able to access the type of whatever object has been received

const kind = type.payload.oneofKind;

if (!kind) {
  throw new Error("No kind specified");
}

// it should be of the correct type or at least a 
// union of RenameTask|TrimmingTask|NoopTask
const oneOfKind = type.payload[kind]; 

// The above doesn't work.
}

with the current type definition I'll have to hard code each possible variant where i'd just like to store the value as is.

Any workaround for this still being type safe?

@timostamm
Copy link
Owner

We would have to include every oneof member as undefined in the union type for that to work:

interface Task_Payload {
    payload: {
        oneofKind: "renameTask";
        renameTask: RenameTask;
        noop: undefined;
        trimming: undefined;
    } | {
        oneofKind: "noop";
        renameTask: undefined;
        noop: NoopTask;
        trimming: undefined;
    } | {
        oneofKind: "trimming";
        renameTask: undefined;
        noop: undefined;
        trimming: TrimmingTask;
    } | {
        oneofKind: undefined;
        renameTask: undefined;
        noop: undefined;
        trimming: undefined;
    };
}

That's not very nice to read, which is acceptable, but it also means that you would have to specify each member as undefined when setting a oneof value:

payload = {oneofKind: "noop", noop: ..., renameTask: undefined, trimming: undefined};
// gets tedious quickly...

I've been passing the entire oneof group around:

function workWithPayload(payload: Task_Payload["payload"]) {
    // ...
}

In my case, it was the right thing to do because it allowed me to easily differentiate the types, which would have been unreliable otherwise .
Would that work for your use case?

@fenos
Copy link
Contributor Author

fenos commented Jul 12, 2021

I see...

However i think it still can be useful to find a way of extracting only the data dynamically.

My use case is essentially to grab the data from the oneof field and store it into mongo

const taskInfo = type.payload[type.payload.oneofKind]

Task.create({
   name: type.payload.oneofKind,
   info: taskInfo,
});

would become:

const data: RenameTask | TrimmingTask | NoopTask

switch(taskInfo) {
 case 'ranameTask':
    data = taskInfo.renameTask;
    break;
 case 'trimming':
    data = taskInfo.trimmingTask;
    break
 case 'noop':
    data = taskInfo.noop;
    break
 default:
   throw new Error("Not supported");
}

Task.create({
   name: type.payload.oneofKind,
   info: data,
});

I just find that the switch statement is completely redundant in this case, and adds more code to manually maintain.
Maybe we can generate a smart function to do exactly that:

TaskProto.oneOfPayload(taskInfo); //  RenameTask | TrimmingTask | NoopTask

@timostamm
Copy link
Owner

However i think it still can be useful to find a way of extracting only the data dynamically.

Agreed.

TaskProto.oneOfPayload(taskInfo); //  RenameTask | TrimmingTask | NoopTask

I think it should be possible to merge this oneof union type into a union of the possible values...

Ref:
https://stackoverflow.com/questions/57334572/typescript-merge-mapped-type-created-from-union-type
https://dev.to/lucianbc/union-type-merging-in-typescript-9al

@fenos
Copy link
Contributor Author

fenos commented Jul 13, 2021

Are you considering including it in the generated code?

@timostamm
Copy link
Owner

Yes, but doing this in the runtime is preferable because of less overall code size.

@jcready
Copy link
Contributor

jcready commented Jul 16, 2021

const getOneOfValue = <
  Oneof extends {},
  Value extends Oneof extends { oneofKind: keyof Oneof }
    ? Oneof[Oneof["oneofKind"]]
    : never
>(oneof: Oneof): Value => {
  const o = (oneof as any);
  if (typeof o.oneofKind === 'undefined') throw new Error('No kind specified');
  return o[o.oneofKind];
};

declare var exampleOneof: {
    oneofKind: "a";
    a: null;
} | {
    oneofKind: "b";
    b: number;
} | {
    oneofKind: "c";
    c: boolean;
} | {
    oneofKind: undefined;
};

const value = getOneOfValue(exampleOneof); // number | boolean | null

@timostamm
Copy link
Owner

Very nice, @jcready.

Are you ok with me adding this to @protobuf-ts/runtime? Or would you like to open a PR?
I think this just needs to return undefined in case oneofKind is undefined.

@jcready
Copy link
Contributor

jcready commented Jul 18, 2021

@timostamm feel free to use the code. I honestly don't think this function is particularly useful. It doesn't narrow the value and the output doesn't really provide a way to narrow further. I mostly posted it because I don't think it needs to be added to protobuf-ts. In my opinion this is something that @fenos can put in his own code if he finds it useful.

I think this just needs to return undefined in case oneofKind is undefined.

I was trying to mimic @fenos's example code wherein the oneof value being set is invariant.

const kind = type.payload.oneofKind;

if (!kind) {
  throw new Error("No kind specified");
}

// it should be of the correct type or at least a 
// union of RenameTask|TrimmingTask|NoopTask
const oneOfKind = type.payload[kind]; 
TaskProto.oneOfPayload(taskInfo); //  RenameTask | TrimmingTask | NoopTask

If you want a version which doesn't throw for the unset case then this is probably a better version for that:

const getOneOfValue = <
  Oneof extends {},
  Value extends Oneof extends { oneofKind: keyof Oneof }
    ? Oneof[Oneof["oneofKind"]]
    : undefined
>(oneof: Oneof): Value => {
  const o = (oneof as any);
  return o[o.oneofKind];
};

declare var exampleOneof: {
    oneofKind: "a";
    a: null;
} | {
    oneofKind: "b";
    b: number;
} | {
    oneofKind: "c";
    c: boolean;
} | {
    oneofKind: undefined;
};

const value = getOneOfValue(exampleOneof); // number | boolean | null | undefined

One issue with either version of the function is that TS doesn't seem to allow me to both: A) Constrain the input type Oneof to an object which contains a oneofKind property AND B) Generically index into the Oneof type to get the union of values. At best I can limit the input type to be an object, but that's it. At least it will be type-safe enough to prevent someone from passing primitives to it.

If you try to constrain the Oneof generic input type to, say, Oneof extends { oneofKind: unknown } then TS thinks that any future references to the Oneof generic type (like in the Value extends bit) can ONLY have the oneofKind property thus making the Value type evaluate to never for any input.

Edit: We can add a bit more type safety to the function(s) by adding another level of ternary operators which will output an unknown type if the input type does not match a protobuf-ts oneof.

type ValueFromOneof<Oneof, UnsetValue = undefined> =
  Oneof extends { oneofKind: keyof Oneof }
    ? Oneof[Oneof["oneofKind"]]
    : Oneof extends { oneofKind: undefined }
      ? UnsetValue
      : unknown;

const getOneOfValue = <
  Oneof extends {},
  Value extends ValueFromOneof<Oneof>
>(oneof: Oneof): Value => {
  const o = (oneof as any);
  return o[o.oneofKind];
};

const getSetOneOfValue = <
  Oneof extends {},
  Value extends ValueFromOneof<Oneof, never>
>(oneof: Oneof): Value => {
  const o = (oneof as any);
  if (typeof o.oneofKind === 'undefined') throw new Error('No kind specified');
  return o[o.oneofKind];
};

const value = getOneOfValue(exampleOneof);             // string | number | boolean | undefined
const setValue = getSetOneOfValue(exampleOneof);       // string | number | boolean

const badInput1 = getOneOfValue({oneofKind: 'wat'});   // unknown
const badInput2 = getSetOneOfValue({oneofKind: null}); // unknown
const badInput3 = getOneOfValue({});                   // unknown

@timostamm
Copy link
Owner

Thank you for looking into this so thoroughly, @jcready.

We already have isOneofGroup(). I've added getOneofValue(), setOneofValue() and clearOneofValue().

They may be helpful in some obscure situations.

@fenos
Copy link
Contributor Author

fenos commented Jul 22, 2021

@jcready sorry for the late reply i've been a busy lately.

Thanks so much for looking into this!
Those are awesome functions!

I'm glad they landed in protobuf-ts

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

3 participants