Skip to content

Identifier values should be of type string#1149

Merged
kriswest merged 3 commits intomasterfrom
clarify_identifier_values_should_be_strings
Feb 6, 2024
Merged

Identifier values should be of type string#1149
kriswest merged 3 commits intomasterfrom
clarify_identifier_values_should_be_strings

Conversation

@kriswest
Copy link
Copy Markdown
Contributor

@kriswest kriswest commented Feb 6, 2024

Adding a note that id field values in context objects should always be of type string (which unfortunately can't be represented in the generated context types.)

If you use additionalProperties: { type: "string" } optional properties in context types derived from context will not compile (as they are implicitlystring | undefined and there is no way of expressing undefined in JSONSchema. While using unevaluatedProperties: { type: "string" } or patternProperties: { ".+": {type: "string" }} just result in the property value being allowed to be any (i.e. [property: string]: any) as both allow for defined properties to differ and hence the general constraint [property: string]: string would conflict

In short, the documentation is correct and the generated types are as close to correct as they can be with the code generation we use. To do better would require a manual change after code generation.
I think this a minor flaw as each subtype that defines id properties can and should define their type as string (as instrument does here). Hence, this restriction is only really applied to non-standard id entries.

@kriswest kriswest added docs Documentation context-data FDC3 Context Data Working Group Context Data & Intents Contexts & Intents Discussion Group labels Feb 6, 2024
@kriswest kriswest requested review from a team and openfin-johans February 6, 2024 10:50
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 6, 2024

Deploy Preview for fdc3 ready!

Name Link
🔨 Latest commit 892ae51
🔍 Latest deploy log https://app.netlify.com/sites/fdc3/deploys/65c2291f5bf1130008b58f76
😎 Deploy Preview https://deploy-preview-1149--fdc3.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Copy Markdown
Contributor

@openfin-johans openfin-johans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - ship it!

@kriswest kriswest requested a review from hughtroeger February 6, 2024 12:39
@kriswest kriswest merged commit ee1a21c into master Feb 6, 2024
@kriswest kriswest deleted the clarify_identifier_values_should_be_strings branch November 13, 2024 18:04
@bingenito bingenito mentioned this pull request Jul 30, 2025
22 tasks
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 docs Documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants