-
Notifications
You must be signed in to change notification settings - Fork 132
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
TimeRange context TypeScript type vs docs inconsistencies #1479
Comments
Hi @hellosmithy, encoding of contexts in JSON and representation in other languages differs - but all other language representations need to be able to convert to and from JSON as the exchange format. We don't say that clearly in the Context overview and should probably clarify. I.e. where we say:
We should probably be saying "... SHOULD be string encoded in JSON according to ..." The JavaScript Date object will stringify to an ISO 8601 encoding, so the Typescript encoding will produce a valid JSON encoding when stringified. However, the example in the TimeRange 2.0 and 2.1 docs pages confuse things by giving an example with the dates initialized as strings - thank you for pointing that out! In the 2.2 version these are JSON drawn from the schema: https://fdc3.finos.org/docs/next/context/ref/TimeRange. We do mention date fields and parsing in Javascript via the Date type in the context overview (https://fdc3.finos.org/docs/context/spec#times) but again are not clear on using Date objects in JavaScript/Typescript. The TypeScript types in ContextTypes.ts are generated via Quicktype from the JSON Schemas and as we list the date fields as Quicktype is also whats behind the particular format of the timerange object. Unfortunately, Quicktypes combines similar objects within a schema, generating a type that could represent any of them, but may be less expressive than the schema itself. Where we've got the following in the schema: "anyOf": [
{
"required": [
"startTime",
"endTime"
]
},
{
"required": [
"startTime"
]
},
{
"required": [
"endTime"
]
}
], That does provide enough information to produce the discriminated union you propose - although we might have been better to use Hence, to adopt that, we'd need to move to manually maintained typescript types for context, which sit well with our long-term goal of opening up the context part of the Standard to additions outside of the (API) version cycle, as we'd planned to focus only on JSON schema submissions and make auto-generated types available. I hope this provides an explanation and background for what you've highlighted, if not a solution. I think based on your comment that we should:
Would that help - or do you think we should go further? Let me know and I can add this to the SWG meeting agenda in January. |
@kriswest thanks for the detailed reply.
I'm still a little unclear on the role of the json schemas in that case. Currently they specify strings for the FDC3/schemas/context/timeRange.schema.json Lines 13 to 24 in bc2fa35
Are you saying the json schema types don't represent the input / output type but rather the serialized "transport" type? If so I guess they would be more targeted to those writing FDC3 platforms rather than consumers. Do other languages also send equivalent |
They do, there's no
https://fdc3.finos.org/docs/next/context/spec Hence, contexts are defined in JSON, which we govern with JSON Schema. A specific FDC3 API binding for a language can represent Context in its own form (e.g. TypeScript Context, .NET Context), as long as they are able to serialize/stringify that form to the required JSON format. This can then be transmitted via various means and parsed at the receiving end into another language-specific representation. In the case of TypeScript types that we publish from the ContextTypes.ts file, FDC3/src/context/ContextTypes.ts Lines 1933 to 1948 in bc2fa35
Yes, although I would say that the Context Part of the FDC3 Standard is focused on that "transport" type, while specific API bindings (i.e. TypeScript, .NET) provide the object types to represent context in that language. There would be nothing stopping them using a string containing the JSON as their representation - although it would not be very easy to work with. Also, please note that the description fields from the Schema are automatically imported as comments on the types, which is why the TypeScript file's comments on startTime and endTime talk about the string encoding. We could refine that description to cause less confusion!
Each language binding works with its own type representing context, which it needs to be able to convert to JSON (rather than javascript) to exchange with other languages. However, that is an implementation detail of the platform - as long as each language interface sends/receives context as defined in its standardized binding it is upto the platform how they handle conversions. Does that answer your questions? If so, what could we be improving in the Standards docs to make this clearer? |
There appear to be a couple of inconsistencies between the published TypeScript types and the docs/json schemas for the
fdc3.timeRange
context.Use of
Date
vsstring
typesFDC3/src/context/ContextTypes.ts
Lines 1933 to 1943 in bc2fa35
The current TypeScript type for
fdc3.timeRange
specifiesDate
types for bothstartTime
andendTime
. Which appears to conflict with the TimeRange docs and the json schema file which both suggest they should be string types encoded as ISO 8601-1:2019:This is also mentioned in the Context Overview
Optional
startTime
andendTime
An additional difference is that the type specifies both
startTime
andendTime
as optional values whereas the docs and json schema specify that:The generated type might be better represented as a discriminated union type to capture this:
The text was updated successfully, but these errors were encountered: