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

Question: Why do RecipientsID and RecipientsObject exist? #853

Closed
bingenito opened this issue Nov 11, 2022 · 6 comments
Closed

Question: Why do RecipientsID and RecipientsObject exist? #853

bingenito opened this issue Nov 11, 2022 · 6 comments
Labels
Context Data & Intents Contexts & Intents Discussion Group context-data FDC3 Context Data Working Group

Comments

@bingenito
Copy link
Member

Question Area

[x] Context Data

Question

Why do RecipientsID and RecipientsObject exist at the code level when the documentation for fdc3.email has a recipients property of fdc3.contact or fdc3.contactList. That doesn't match the code. Contact has an id property of type ContactID, ContactList has a contacts property of Contact[] but RecipientsObject has an optional id property of type RecipientsID which has a property contacts. This is three different type of types to look for when implementing this.

@kriswest kriswest added context-data FDC3 Context Data Working Group Context Data & Intents Contexts & Intents Discussion Group labels Nov 29, 2022
@kriswest
Copy link
Contributor

kriswest commented Dec 5, 2022

@bingenito This appears to be the fault of the Typescript generator which is taking this oneOf:

"recipients": {
"oneOf": [
{ "$ref": "contact.schema.json#" },
{ "$ref": "contactList.schema.json#" }
]

and creating RecipientsObject from the union of the two types (although treating it more like an intersection) + RecipientsID as a copy of ContactID to use on the new type.

The result is a set of types that probably do validate most contexts correctly, but would also allow a weird hybrid that's not valid. They should really be expressing a union instead - and based on our previous chats about the difficulty of union types in other languages might have been better defined without the union (always use ContactList). That in turn always prompts me to think about whether we should be negating the need for the list types as well will some form of generics support...

The Type generation from JSON Schema is performed by quicktype. I think someone is going to need to take a look at any options available within quicktype to see if we can get better output from it - or we consider moving to manually maintained types to give us more control.

@kriswest
Copy link
Contributor

kriswest commented Dec 5, 2022

@bingenito BTW have you considered attempting to generate your C# types from the json schema via quicktype? I'm wondering if it will take a different approach for C# code generation.

@kriswest
Copy link
Contributor

kriswest commented Dec 5, 2022

Using the quicktype app I'm getting the same behavior:
image

C# output creates a recipients class still that is a bizarre merge of the two available types (Contact and ContactList):
image

@bingenito
Copy link
Member Author

bingenito commented Dec 16, 2022

@bingenito BTW have you considered attempting to generate your C# types from the json schema via quicktype? I'm wondering if it will take a different approach for C# code generation.

I wanted to have handlers and callbacks be based on a common base type Context with type property while still allowing fully typed ID properties, so went with went with a more manual route involving a base Context<T>, so something like Instrument is Intrument: Context<InstrumentID>. I'll reconsider a generator if it makes things easier because writing these by hand is not fun (although it allowed me to see the issues I've entered in).

@mistryvinay
Copy link
Contributor

hi @bingenito did you need any more support with the above? I have added this issue to this weeks discussion group meeting (#903) if you would like to discuss further.

@kriswest
Copy link
Contributor

closed due to inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Context Data & Intents Contexts & Intents Discussion Group context-data FDC3 Context Data Working Group
Projects
None yet
Development

No branches or pull requests

3 participants