Stream: javascript
Topic: typescript support
Brian Kaney (Nov 21 2020 at 16:59):
Hi all, we are working toward having typescript definitions for STU3, R4, (and eventually R5).
The original effort can be tracked here in this PR: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/49574. As you can see in the conversation, this will take a few steps; the first is to properly version the existing @types/fhir to be 3.0. That has started here: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/49719
Anyway, if you are interested in typescript, feel free to comment or otherwise contribute towards this effort!
Gino Canessa (Nov 23 2020 at 15:50):
Hi Brian, if you are looking for some base definitions to start from, I have a set posted at: https://github.com/microsoft/fhir-codegen/tree/main/generated . In general, the project can output various languages, with the TypeScript output being done by this file.
Happy to make changes or accept PRs.
Brian Kaney (Dec 16 2020 at 14:56):
@Gino Canessa Awesome! I'll take a look. I forked an existing generator and modernized it a bit here: https://github.com/Vermonster/fhir-dts-generator/tree/r4 and am working toward versioning an r4 at DT soon.
Brian Kaney (Jan 24 2021 at 22:50):
@Gino Canessa - I am looking at the MS codegen, very nice! I would like to use the output of this for my own projects and also publish to DefinatelyTyped (now we have major FHIR version tracking setup there).
One nice change -- in the event the property is of type code, and has a required binding, it would be great to take advantage of the Enums that are produced instead of having it a string
.
For instance, this example (set use?: HumanNameUseCodes
instead of use?: string
).
I am trying to figure where in the source code to look for this, any pointers?
Gino Canessa (Jan 25 2021 at 16:34):
Hi @Brian Kaney , I'm glad it's useful!
This is done in the Firely output, so one good place to start is there. If you look at Language\CSharpFirely2
line 1402 (WriteCodedElement
), the function starts with a form of the checks you would want to use. In the Language\TypeScript
file, I'd say the easiest place to match it would be around line 549 (WriteElement
). I don't have all the context loaded in my head, but I'd be happy to spend more time on it either later in the week or next (depending on WGM schedule).
As a note on language development with codegen, if you regenerate any of the 'standard' files (checked in via the generated
directory in the root of the repo) you can use git to see the differences your changes caused.
I'll continue checking the thread here as possible, so if you have any other questions please feel free to ask!
Brian Kaney (Jan 30 2021 at 21:30):
@Gino Canessa -- Here is an attempt - https://github.com/microsoft/fhir-codegen/pull/45
I also have changes to make the code work better cross-platform. There are some strange path defaults that are windows -specific. LMK I can make another PR.
Gino Canessa (Feb 01 2021 at 16:24):
Thanks @Brian Kaney ! I'll take a look shortly.
Gino Canessa (Feb 01 2021 at 16:27):
And yes, I'd love a PR for the path changes as well. I try to remember to test in WSL periodically, but apparently haven't in quite some time.
Brian Kaney (Feb 16 2021 at 21:09):
Hey @Gino Canessa - I found a bug. When there is a choice type with a 1..1
, such as UsageContext.value[x]
(https://www.hl7.org/fhir/metadatatypes.html#UsageContext), the generated interface is below -- which specifies all types are required:
I think we need this sort of pattern (e.g. "RequireOnlyOne") https://realfiction.net/2019/02/03/typescript-type-shenanigans-2-specify-at-least-one-property. I can work on another PR.
Brian Kaney (Feb 16 2021 at 21:11):
(or a hack to make them all optional, which is the current situation -- https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/fhir/index.d.ts#L2385-L2402)
Josh Mandel (Feb 16 2021 at 21:12):
All optional is probably the best reflection of the FHIR requirements (which even for 1..1
elements allows you to provivde an extension instead of one of the choices).
Brian Kaney (Feb 16 2021 at 21:13):
@Josh Mandel well that's simpler to implement. Should we do that (make them all optional)?
Josh Mandel (Feb 16 2021 at 21:16):
If Gino agrees :)
Josh Mandel (Feb 16 2021 at 21:21):
(BTW, the hack you linked to is awesome -- great illustration of how fun/productive the type system is.)
Gino Canessa (Feb 16 2021 at 21:23):
Yeah, great catch!. Let me take a quick peek - making that situation optional should be a quick fix.
Brian Kaney (Feb 16 2021 at 21:29):
Thanks @Gino Canessa -- I am running through the examples to create tests for the generated types...
Another problem is contained resources have a resourceType
property for serialization in JSON. But this is not part of the resource interface. So all these fail:
image.png
Gino Canessa (Feb 16 2021 at 21:37):
@Brian Kaney - PR 47 has the optionality change, if you'd like to take a peek before I merge it in (has the change to the TS language and each of the generated files).
Brian Kaney (Feb 16 2021 at 21:41):
@Gino Canessa - Awesome!
Though more testing now, I think my inline of codes caused problems (which I am finding though testing). In here https://github.com/microsoft/fhir-codegen/blob/main/src/Microsoft.Health.Fhir.SpecManager/Language/TypeScript.cs#L590 I assumed element.Codes
is exhaustive. It is not (see the generated list for Observation.status
). So I think we either need to remove this, or figure out how to get the exhaustive list in this case.
Gino Canessa (Feb 16 2021 at 21:43):
IIRC, I had issues with that at the time and punted on Resource
- defining the field as type string
prevented the resources from defining the literal.
One approach is to define enums (similar to the 'data_types_*' Codings) and then explicitly allow those values (nice and explicit, but quite verbose).
Another approach would be to add the field to Resource
with something like string|any
.
Thoughts?
Brian Kaney (Feb 16 2021 at 21:47):
This is the approach used presently -- https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/fhir/index.d.ts#L26665
Brian Kaney (Feb 16 2021 at 21:50):
In the codegen approach, I think we could make it a property of Resource as a code/string , and then limit to the list of resource types?
Gino Canessa (Feb 16 2021 at 21:55):
Hmm.. in that file, ResourceBase
has a resourceType?: code;
element, which is then not included on the actual resources (e.g., Patient
does not have anything that indicates the resourceType
must be present and must be the literal 'Patient').
If we want to do something similar, I'd lean towards defining the union of the literals as allowed on resource, then each individual on the type.
Gino Canessa (Feb 16 2021 at 21:55):
For the codes (e.g., Observation.status
, I should have the proper expansion loaded, give me a minute to see.
Brian Kaney (Feb 16 2021 at 22:05):
right, I like how you have the resourceType guard. I like the idea of adding the exhaustive set of literals on Resource.
Gino Canessa (Feb 16 2021 at 22:17):
Codes was an easy fix - this commit has the code changes and new generated files. It uses the expansions instead of the resource-embedded versions.
I'm looking at the resourceType changes, so haven't made another PR yet.
Gino Canessa (Feb 17 2021 at 17:30):
@Brian Kaney - in working through the implementation, I'm concerned about the performance implication of putting the literals together. Doing string comparisons to a list of >100 literal values isn't great for performance.
I've added the field as any
on Resource for now - it's in PR 48. If this works for you I'll merge it in. If it doesn't, please let me know.
Thanks!
Brian Kaney (May 30 2021 at 20:25):
Okay, if anyone is interested in typescript, I pushed a commit to DT that has R4 (current), and versions R3 and R2 -- https://github.com/DefinitelyTyped/DefinitelyTyped/pull/53511 -- please comment so we can get this pushed through (the DT people might not be familiar with FHIR).
Brian Kaney (Jun 02 2021 at 18:43):
Done!
You can npm i -D @types/fhir
and enjoy 3 namespaces: fhir2
, fhir3
, and fhir4
in all your favorite typescript project.
Thanks @Gino Canessa and others for help! I have a talk next week at DevDays about this, and will post some examples online.
Gino Canessa (Jun 02 2021 at 19:43):
Thank you for all of your hard work! This is great, and I am looking forward to the session!
Ilya Beda (Jun 15 2021 at 21:07):
I have review existing type annotation and find out that Bundle is not generic.
In type generator, my team developed https://github.com/beda-software/aidbox-ts-generator it is generic.
Both Bundle https://github.com/beda-software/sdc-ide/blob/master/shared/src/contrib/aidbox/index.ts#L1361
and BundleEntry https://github.com/beda-software/sdc-ide/blob/62d3a74b5ac6e5eec6e51ee3edb4ed2f01419486/shared/src/contrib/aidbox/index.ts#L1385
are generics.
I decided to contribute to https://github.com/microsoft/fhir-codegen/ to add this useful feature and find out that it is written with C# :scream:
I am not sure what used under the hood in fhir-codegen. It seems that it works with pure strings. Please correct me if I am wrong here.
In aidbox-ts-generator we are using https://ts-morph.com/ to generate TypeScript AST.
It is a simple and powerful way, and you can see how simple and elegant implementation may be.
It is only 443 lines of code.
So why are we using the C# version fot the type annotation generator?
Gino Canessa (Jun 15 2021 at 22:07):
Hi Ilya, there's no conspiracy here... it uses C# because that's what I started the project in =). The code generator is a generic tool that reads any version of the FHIR specification (and coming soon: IGs), normalizes it, and exports code - there are a several languages exported today, as well as some non-language things like the Info files or visualization experiments with Cytoscape.
I added TypeScript when I needed it and couldn't find anything usable (particularly because I was trying to use Connectathon preview builds, which I failed to find any tooling around). Brian picked it up and did the work (I helped! =) to modify it for DT. If someone wants to rewrite all of the generation code in TS, I can provide pointers - but it's a fair bit of work for all the backend stuff and I don't plan to rewrite it for every language I use. Everything is cross-platform, so there shouldn't be any barriers there.
I'll note that I chose this method over static transformations (e.g., templating) because the experience I saw with templating over time was that it became unusable (e.g., the templates for the C# library over 10 years of changes). I wanted something that completely separated the language part from the specification part, and would have either needed to create my own IL or do something like this :shrug:
In that project, the language export is separate from all the specification stuff, and is typically a single file. For TS, it's all in Language/TypeScript.cs, which comes in at a total of 765 lines and includes a fair bit of boilerplate and pretty formatting.
I have a fair bit of documentation, though it is slightly out of date (typing this made me discover that I lost the automatic update part when I changed computers - so that'll be fixed soon). If it would be helpful to go through it in detail, I am happy to do that as well.
While PRs are preferred, I'm also fine with making changes based on discussion here or GH requests. If there's a 'new' version for an object that I can diff with the existing output, I can work through changes. I'll note that PR's are preferred because I have a pretty full plate right now. Also, since these definitions are being used in an 'official' way, we can either make branches to test modifications or fork the existing TS language file into a new language to test until we want to update the original.
Ilya Beda (Jun 16 2021 at 07:31):
Gino, thank you for the detailed explanation. I clarify a lot for me.
As I mentioned, I am not familiar with the C# ecosystem. Is it possible to upload the compiled version of fhir-codegen to npm to allow launch it throw npx to get a similar experience to all other nodejs ecosystem?
Will it require mono runtime to be installed in the system first? Are there any possible backward compatibility issues with such an approach? Is there something similar to https://www.graalvm.org/ for .net world?
As I mentioned on DevDay it is important to move the type annotation generation process on the application layer.
Instead of using generic fhir4 from npm, developers need a profiled version of FHIR.
Profiles are defined on the app layer, so the codegen process should happen there as well.
So, the goal for TypeScript annotation generator is not just to generate a set of predefined fhir2, fhir3, fhir4, etc.
t is highly important to give developers a simple way to create type annotation for the app runtime.
Finally, it all my thoughts, please feel free to correct me if I am wrong somewhere, and let's continue the discussion.
@Brian Kaney I would like to hear your thought as well.
Brian Kaney (Jun 16 2021 at 16:40):
@Ilya Beda -- a few thoughts:
-
I also don't regularly use C#, and am on mac. I use asdf (https://asdf-vm.com/) to manage all my languages across all my projects (here are all the languages https://github.com/asdf-vm/asdf-plugins), and it also works fine for dotnet (https://github.com/emersonsoares/asdf-dotnet-core)
-
I found this command handy for running the codegen project (from the root of the git repo):
> dotnet build
> dotnet src/fhir-codegen-cli/bin/Debug/netcoreapp3.1/fhir-codegen-cli.dll --official-expansions-only true --export-types "primitive|complex|resource" --language TypeScript
See the documentation for argument reference - https://github.com/microsoft/fhir-codegen.
- You could package up the output of codegen to make and npm module (adding package.json, etc). I have a GH project that I will push later this week that I used to packages up special for DT. This includes generating tests from all the examples in the FHIR spec. Then does linting, testing, and then creates this structure with these files so making future PRs could be simplified: https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/fhir
Gino Canessa (Jun 16 2021 at 18:01):
No problem Ilya!
Regarding C#, there are similar tools in general - each community ends up building roughly the same things, but with different priorities. So, some will be "better" and some will be "worse" in a completely subjective way.
As I did during DevDays, I disagree that the generation needs to be in the application layer - or at least that it is useful to replicate that work now. In order to do the generation, we need local access to the specifications, the expansions, and eventually any IGs in question. Especially if multiple versions are supported (and the intention is to continue into the future), there is a lot of plumbing work that I think of as infrastructure tooling. That is the same reason why this tool works off of the built versions of the specifications instead of the definitional inputs - there is a lot of work that people are already doing and I can leverage that instead of repeating the effort. It may be nice one day in the future that to have the same code that builds the Java definitions also export multiple languages, but I am not worried about it now.
That said, I am aware that each of us also has different needs and priorities, so I will also try to assist anyone if they do want to take on the project of doing an equivalent project from the ground up. But I consider part of that suggesting "don't do it" =). For some short and recent examples: 4.0.1 changed some internal type tracking (JSON/XML type to FHIR type), which broke code gen at that time; the inheritance models were changed, which broke a bunch of things again; then R5 (and now R4B) moved the build input from the spreadsheets to StructureDefinitions, which had a lot of little implications, etc.. None of these are related to any particular language, they are just development taxes that need to be paid. In this project, those changes were isolated and fixed without any changes to the language exporters.
I hope the goal is to move beyond 'pile of generated types' and get to a proper generic FHIR library (of which I believe there are several in progress, but to my knowledge there isn't a clear 'winner' yet). In C# land, the generated code is used as a part of the larger library created by Firely (in fact this was swapped in for another system, which was already the second or third, if I remember correctly). Much of the functionality in a library is not something that should be generated - this project started as me trying to move that boundary a bit in one direction.
In that model, tailoring the core definitions to look a way that works in a library makes sense, so things like generics will be good across-the-board improvements. I would expect library usage to be adding an NPM for the core (e.g., FHIR R4), and provide some path for developers to work with IGs that does not involve large or complex pipelines. There may need to be additional tools that extend the already generated code (e.g., something in TS contained in the library), a service that lets you grab the definitions you want (e.g., download via FHIR registry, Simplifier, a standalone site, etc.), or some other plumbing.
Overall, I am happy to contribute as needed. I am quite happy to defer on what is needed to some of the many people here who can speak better to that direction. But I will note that I also do not plan to redo several years worth of work to change the language.
Joshua Kelly (Jun 16 2021 at 18:29):
Brian Kaney said:
- I found this command handy for running the codegen project (from the root of the git repo):
> dotnet build > dotnet src/fhir-codegen-cli/bin/Debug/netcoreapp3.1/fhir-codegen-cli.dll --official-expansions-only true --export-types "primitive|complex|resource" --language TypeScript
Thanks, I couldn't get this working when I, similarly to @Ilya Beda tried earlier in the week to build the project and couldn't figure out how to actually execute it (also a Mac user). @Gino Canessa would you accept a PR from me that basically just adds the above under a 'development' heading?
Gino Canessa said:
There may need to be additional tools that extend the already generated code (e.g., something in TS contained in the library), a service that lets you grab the definitions you want (e.g., download via FHIR registry, Simplifier, a standalone site, etc.), or some other plumbing.
We would need this in order to, for example, write a TS native Validator. To be clear though are you agreeing that a PR to add, in this specific case, to make Bundle generic in the TS output?
Gino Canessa (Jun 16 2021 at 19:09):
@Joshua Kelly
Re: documentation: of course! I hadn't realized the deficiency in the documentation as it was. If you add something I will try to keep it up to date; I finally got around to replacing my 17" MBP with an M1, so I can start testing there myself periodically as well.
Re: Generics: yes, with the caveat that I don't want to break Brian's version that's in DT. So if he's on board with the change we can modify the existing TS output and if not, we can add an new language output for it (e.g., TS_Generic, etc.).
Generally speaking, I try not to step on toes of people maintaining a language. Since it's about 10 seconds of work to duplicate it as a new 'language' for export, that's an easy alternative.
Brian Kaney (Jun 16 2021 at 19:55):
Re: Generics: yes, with the caveat that I don't want to break Brian's version that's in DT. So if he's on board with the change we can modify the existing TS output and if not, we can add an new language output for it (e.g., TS_Generic, etc.).
:+1: I think we should do this, as the types were just released is early enough to support some churn. I can see about including notes to describe this change.
Joshua Kelly (Jun 17 2021 at 14:38):
Gino Canessa said:
Joshua Kelly
Re: documentation: of course! I hadn't realized the deficiency in the documentation as it was. If you add something I will try to keep it up to date; I finally got around to replacing my 17" MBP with an M1, so I can start testing there myself periodically as well.
Done - sent a PR. I successfully reproduced the TS build on a Mac.
Here's a direct comparison of Bundle
between the two implementations (fhir-codegen first):
export interface Bundle extends Resource {
resourceType: 'Bundle';
entry?: BundleEntry[];
identifier?: Identifier;
link?: BundleLink[];
signature?: Signature;
timestamp?: string;
_timestamp?: Element;
total?: number;
type: ('document'|'message'|'transaction'|'transaction-response'|'batch'|'batch-response'|'history'|'searchset'|'collection');
_type?: Element;
}
export interface Bundle<T extends Resource = any> {
readonly resourceType: 'Bundle';
id?: id;
meta?: Meta;
entry?: Array<BundleEntry<T>>;
identifier?: Identifier;
implicitRules?: uri;
language?: code;
link?: BundleLink[];
signature?: Signature;
timestamp?: instant;
total?: unsignedInt;
type: code;
}
Supporting generic on Bundle
and BundleEntry
makes sense to me. So does readme
on any resourceType
fields for that matter. If I write a PR to support would it would be against fhir-codegen
directly right?
Joshua Kelly (Jun 17 2021 at 14:54):
@Ilya Beda apparently it is possible to distribute cross platform binaries inside of an npm package (sqlite3 is apparently installable via npm and is a reference way to do this). So conceivably we could make it npx
-friendly.
Brian Kaney (Jun 17 2021 at 15:13):
@Joshua Kelly - does this accommodate polymorphic bundles? NM: I was confused.
Abbie Watson (Jun 17 2021 at 15:45):
fwiw, the NodeOnFhir implementation is much more similar to the first example (fhir-codegen) which is more EcmaScript-ish. My understanding from the Meteor ecosystem is that the JVMs are broadly targeting EcmaScript as their underlying implementation, and TypeScript just gets compiled down to that.
(I hope we proceed cautiously with encouraging inline typing. Just because we can, doesn’t always mean we should.)
Brian Kaney (Jun 17 2021 at 15:46):
We could make a docker image (e.g. with dotnet and the CLI installed), then invoke with a local volume mount. I don't really see compelling benefits having an npm wrapper for fhir-codegen.
Joshua Kelly (Jun 17 2021 at 20:32):
For purposes of discussion of generics in the definition specifically, generic support lets you constrain the entries of a Bundle, like so (adding some Resource other than Patient or Practioner would cause it to fail, and specifying none at all is fine).
const bundle: Bundle<Patient | Practitioner> = {
resourceType: "Bundle",
type: "collection",
entry: [
{
"resource": {
"resourceType": "Patient",
"id": "patient-01"
}
},
{
"resource": {
"resourceType": "Practitioner",
"id": "practitioner-01"
}
}
]
}
This doesn't feel like anything but a nice additive improvement to me since it doesn't break using them without this annotation.
@Abbie Watson agree that ES target is the "prize" with TS in general. I see it as a way to build great ES tools. I think having a really holistic implementer story that considers how to effectively use types throughout a project and in different ways is important to get right.
Joshua Kelly (Jun 17 2021 at 20:45):
Here's a WIP commit to support this @Ilya Beda: https://github.com/Automate-Medical/fhir-codegen/commit/8f61e67eb23a814d84546180420c7c97f76b263a - tho @Brian Kaney you may have better idea than just hacking the exportName check inline here.
Ilya Beda (Jun 17 2021 at 22:48):
:thumbs_up: @Joshua Kelly
I added a small note https://github.com/Automate-Medical/fhir-codegen/commit/8f61e67eb23a814d84546180420c7c97f76b263a#r52324266
please review.
I see that T is used as a type of resource inside BundleEntry in example.ts but I can find these changes in the source code.
I also think that these changes are fully backward compatible and may be safely applied.
Joshua Kelly (Jun 22 2021 at 17:30):
I've opened up a new PR on fhir-codegen based off of the feedback from the initial WIP commit above: https://github.com/microsoft/fhir-codegen/pull/66
Gino Canessa (Jun 22 2021 at 17:44):
Good deal, a few of questions:
- I assume we want this everywhere there is a
FhirResource
today (e.g.,Parameters.parameter.resource
, etc.), correct? - Where do we want the generic annotations? Is it just on the element and the resource (
Bundle<T>
andBundle.entry[].resource<T>
) or each step of the way (e.g., also annotateBundle.entry<T>
for consistency). - For
contained
, I assume we do not want to bubble up at all (e.g., we don't wantEncounter<Patient>
or anything crazy like that). But, we still wantResource.contained<T>
, correct?
Josh Mandel (Jun 22 2021 at 22:03):
I'm not clear on use cases for constrained .contained
, but if we want this, it kind of implies Resource<ContainedType>
, doesn't it? As in your Encounter<Patient | Organization>
to describe an Encounter whose .contained[]
can include Patients or Organization resources?
Gino Canessa (Jun 22 2021 at 22:12):
Yes, that's why I'm asking about it. I can see the value in Parameters.parameter
(though not as clear to me in Parameter
itself). For the same reason I can see the value in Encounter.contained<Patient>
, but I don't really like the idea of making Encounter<Patient>
since it's such a rare use case to start with (and I think will cause confusion, especially to new devs).
Brian Postlethwaite (Jun 23 2021 at 06:10):
I don't know that i'd use the generic in the bundle either, as with searches and lots of other types, there are more than 1 type in there, such as with _include, or the outcomes that can be included.
And particularly OperationOutcome should always be in the options for the Bundle I'd be wanting...
Josh Mandel (Jun 23 2021 at 13:59):
For sure the correct use of bundle generics in search would involve expressions like Bundle<Encounter | Patient | OperationOutcome>
Ilya Beda (Jun 25 2021 at 11:56):
I don't think that generics should be used everywhere when FHIRResoruce is used.
Bundle and BundleEntry is a quite different situation.
One of the most common use cases is when you are using FHIR search API to get a lit of specific resources.
I am using it like this
https://github.com/beda-software/aidbox-react/blob/master/src/services/fhir.ts#L154
const patients = await getFHIRResources('Patient');
I don't need to specify a type for the patients
variable. Based on the getFHIRResources
definition TypeScript will automatically calculate it as Bundle<Patient>. It simplifies code a lot and keeps it typesafe.
When I use _include or _revinclude I have to set type explicitly like this.
const patients = await getFHIRResources<Patient | Encounter>(
'Patient', {_revinlude: 'Encounter:patient'});
Josh Mandel (Jun 25 2021 at 13:08):
But there is more complexity lurking here. From discussion above, it is at least clear that these search cases need to account for operation outcomes, which might always appear alongside resources of the requested type in the search response bundle.
Joshua Kelly (Jun 25 2021 at 15:27):
Joshua Kelly said:
I've opened up a new PR on fhir-codegen based off of the feedback from the initial WIP commit above: https://github.com/microsoft/fhir-codegen/pull/66
I found a nice implementation example yesterday when using generics on Bundle to support typing the prefetch requests in CDS Hooks land. Also uses the CDSH types @Brian Kaney released. We can type the expected prefetch structure:
import { CDS } from "../src";
interface PatientPrefetch {
patient: fhir4.Patient;
// upcoming in @types/fhir - suggested in PR and this thread
hemoglobin: fhir4.Bundle<fhir4.Observation>;
}
function hookRequestResponse(request: CDSHooks.HookRequest<PatientPrefetch>) {
// typing is inferred here, but for explicitness...
const { patient, hemoglobin }: PatientPrefetch = request.prefetch
// function can respond to hook and has access to typed Observation in the Bundle
// returned in the hemoglobin key - versus having just the flat Bundle typing
}
Brian Kaney (Jun 25 2021 at 20:48):
@Joshua Kelly looks nice - should I add generic support to https://github.com/Vermonster/smart-typescript-support/blob/main/types/cds-hooks/index.d.ts ? I was thinking about representing the types for each hook context.
Also, should I make this in DT instead of the current npm package?
Ilya Beda (Jun 26 2021 at 07:31):
Error handling is another tricky part. Thank you @Josh Mandel for raising this up.
I am using the RemoteData approach for API error handling.
Initially, it comes from elm-lang. Here is a blog post about this approach http://blog.jenkster.com/2016/06/how-elm-slays-a-ui-antipattern.html
Here is my RemoteData adaptation for typescript https://github.com/beda-software/aidbox-react/blob/master/src/libs/remoteData.ts
Then I use it in service to fetch data:
https://github.com/beda-software/aidbox-react/blob/master/src/services/service.ts#L18
Finally, if the server returns OperationOutcome it will be captured with Failure
. In my previous example the type of patients is like this.
const patients:RemoteData<Bundle<Patient | Encounter>, OperationOutcome> =
await getFHIRResources<Patient | Encounter>(
'Patient', {_revinlude: 'Encounter:patient'});
Josh Mandel (Jun 26 2021 at 14:33):
Neat -- like this fancy Either ;-) but to be clear I wasn't talking about operation outcomes that represent errors. Oftentimes a FHIR API server will include an operation outcome as a bundle entry in a successful search response providing some additional detail or context -- that is an informational status OperationOutcome rather than than an error.
Ilya Beda (Jun 27 2021 at 11:59):
Hmm, I don't face such behaviour previously. Could you please provide an example of a request that returns such a result ?@Josh Mandel
Josh Mandel (Jun 27 2021 at 14:30):
Every Epic instance.
Josh Mandel (Jun 27 2021 at 14:32):
https://github.com/jmandel/my-health-data/blob/4dbc49f184cd5aabc9c7eea26060d1e212dc82ad/my-data/2019-03-01/data-from-uw/laboratory.json is old but handy -- even if the result count was >0 this OperationOutcome could still be present
Brian Kaney (Jun 29 2021 at 16:54):
Yea, personally I don't see a ton of benefit from using generics with bundles to be honest, but others seemed to, and it's optional.
I find that I am usually dealing with bundles from an external server. And I use a type predicate guard when processing these bundle entries - because as you mention they are polymorphic (including the occasional OperationOutcome).
Brian Kaney (Jun 29 2021 at 16:59):
@Ilya Beda - In one example, if you do a Batch bundle for a GET, POST, PATCH, etc, it is possible that one/some could fail, while the others are okay. In this case the return bundle could have OperationOutcome entry(ies).
Ilya Beda (Jun 29 2021 at 19:38):
Oh.
I have a quite different experience.
Usually, I got an empty bundle instead of OperationOutcome with Resource request returns no results
warning.
Also, I prefer to use a transaction bundle for bulk updates instead of the batch.
The transaction fails if even one entry fails. So, I got OperationOutcome instead of Bundle.
Brian Kaney (Jun 29 2021 at 20:17):
Also, I prefer to use a transaction bundle for bulk updates instead of the batch.
The transaction fails if even one entry fails. So, I got OperationOutcome instead of Bundle.
What if you want to run a several search or read operations, there would be no need for a transaction then....
Ilya Beda (Jun 30 2021 at 08:34):
I am using Aidbox and it doesn't fire OperationOutcome in case of missing data, as Epic did in Josh's example.
All issues like wrong search parameters handled in the dev environment, so I don't care about these cases.
Read, search operations are OperationOutcome free in my case =)
Josh Mandel (Jun 30 2021 at 13:47):
I'm not saying there are no scenarios where you can ever be sure about what's in a bundle. I'm saying that according to the base FHIR specification (and consist with real world server behaviors) you need to be ready for such things. So when we think about what goes into a general purpose library or type definitions, those are the considerations we need to make.
Ilya Beda (Jul 01 2021 at 06:20):
You always can use OperationOutcome as one of the elements of union type.
Bundle<Patient | OperationOutcome>
Even if you don't specify it explicitly, the generic's default should be FhirResource. It is a union type that contains all resources including OperationOutcome.
So, it covers all possible cases I think.
Josh Mandel (Jul 01 2021 at 13:27):
Agreed, I used that same example above. I have no concern about covering Bundle.entry.resource in a Bundle generic. I don't think I see the rationale for Resource.contained or Parameters.resource.
Gino Canessa (Jul 01 2021 at 14:11):
Makes sense - my concern was consistency. It feels odd that we use a generic in one place but not other areas that have the same definition. I'll go through what's open shortly.
Brian Kaney (Jul 01 2021 at 14:46):
@Gino Canessa -- I tend to agree. Is there a reason @Ilya Beda we wouldn't do the same to contained
or in Parameter? Maybe there are other cases as well...
Brian Kaney (Jul 01 2021 at 14:56):
I just read @Josh Mandel comment -- why not support generics for contained or Parameter? Or to put it a different way, why is Bundle unique?
Josh Mandel (Jul 01 2021 at 15:53):
Both of these are used in widely variable ways that may be pretty hard to predict, though in heavily profiled environments, your mileage may vary. If you make contained generic, you wind up making every Resource generic. I'm not arguing against; just not clear on the benefit, is all.
Gino Canessa (Jul 01 2021 at 21:59):
FYI: I created a new PR (67) which builds on the one that was there. There were a few issues that I cleaned up and then I regenerated the TS files for review. It should have tagged the four of you, but :shrug:.
It has generic support from Bundle
down to Bundle.entry.resource
, and defaults to FhirResource
if nothing is specified. It is also fairly straightforward to add more (just more entries in the dictionary here), if we like this and identify areas we want it later.
I briefly considered generics for choice elements (e.g., Patient.deceased<boolean|dateTime>
), but I believe doing that requires changing everything from interfaces to classes so that we can customize serialization / parsing. Definitely not a 'today' thing.
Joshua Kelly (Jul 20 2021 at 02:12):
I think we should add an enum type for FHIR Resource different from the currently defined FhirResource, which is a union type of all of the individual resource types. I think this should be purely additive, but here's the difference:
Existing:
export type FhirResource =
Account
|ActivityDefinition
|AdverseEvent
New addition:
export enum FhirResourceList {
Account = "Account",
ActivityDefinition = "ActivityDefinition",
AdverseEvent = "AdverseEvent",
Why is this useful? There are lots of places where I'm finding that I need to enumerate the list of string Resource types. Here's one neat example I found recently in being able to define 1 liner template literal type definitions for SMART clinical scopes (v1 and v2 - tho the v2 example isn't totally complete:
export type ClinicalScopeV1 = `${"patient" | "user" }/${fhir4.FhirResourceList | "*"}.${"read" | "write" | "*"}`
export type ClinicalScopeV2 = `${"patient" | "user" | "system"}/${fhir4.FhirResourceList | "*"}.${"c" | "r" | "u" | "d" | "s"}`
This is really expressive because the type almost acts like a regex on the string content, and having the enum makes it possible (this does not work with the union type in fhir4.FhirResource).
Other reasons why an enum would be useful include that you can't enumerate union type values, but obviously enums can be. That's helpful especially in cases where you're using the type declarations to auto generate code like RESTful routes.
- Is there interest/support for this as PR on the mainline fhir-codegen TS output? @Brian Kaney @Gino Canessa
- Do we like the name FhirResourceList? Are there other language library reference examples that deal with this issue?
Josh Mandel (Jul 20 2021 at 02:35):
Thanks for the clear explanation of your use case here! Am I right in thinking it could also be met with something like;
type FhirResourceType = "Account" | "ActivityDefinition" | ....
and if so, are there reasons to prefer one approach over the other?
Joshua Kelly (Jul 20 2021 at 03:36):
Josh Mandel said:
Thanks for the clear explanation of your use case here! Am I right in thinking it could also be met with something like;
type FhirResourceType = "Account" | "ActivityDefinition" | ....
and if so, are there reasons to prefer one approach over the other?
Yes, this is the string union type. It would work for the template literal examples. However, union types can't be enumerated. The enum type is somewhat unique in TypeScript in that the compiler will allow you to write run-time enumeration.
An example where this was helpful to me recently:
const restResources: fhir4.CapabilityStatementRestResource[] = Object.values(fhir4.FhirResourceList).map((resource) => {
return {
type: resource,
...defaultRestReourceCapability
}
}
})
From what I understand, enum
is feature compat with what a string union is at a type level but has the added benefit of runtime enumeration.
Josh Mandel (Jul 20 2021 at 04:04):
Thanks! And the proposal here is to add an enum supporting these use cases, without changing how existing types are modeled (i.e., existing definitions would stay the same)? If so, it sounds good and useful to me.
Ilya Beda (Jul 20 2021 at 09:22):
Looks good to me as well.
Brian Kaney (Jul 20 2021 at 19:09):
this random blog post rates union vs enum a tie gives the edge to union actually -- https://blog.bam.tech/developer-news/should-you-use-enums-or-union-types-in-typescript
Brian Kaney (Jul 20 2021 at 19:12):
I recommend a test to see if either has any (code editor) performance consequences. The FHIR spec is pretty big, and this factor might be the priority.
Josh Mandel (Jul 20 2021 at 19:17):
Re: that blog -- it's a great write-up. Looking at the use cases here though
- extensibility isn't critical given that a FHIR release corresponds to a fixed set of resource types
- iteration over unions only works if you declare a const array in addition to the type
- switch statements shouldn't be relevant here because switches would presumably be over
someLocalResourceVar.resourceType
as already modeled, and not over this new union-or-enum type.
--> if iteration is the primary use case (and others are already met through existing means), enums have a slight edge.
Josh Mandel (Jul 20 2021 at 19:17):
Agreed that looking for perf issues (in editor and/or tsc step) is a key step
Gino Canessa (Jul 22 2021 at 21:18):
Ok, have PR 71 for review. I'm trying to sort out permissions to actually be able to tag people as reviewers, but since the repo is MS-owned it's a bit of a challenge.
I have not done performance testing on this yet. It would be trivial to add both options if that is easier for someone to use and compare.
In response to an earlier question, yes other languages do something like this. In C# we generate two dictionaries, one string, type
and one type, string
so that it is easy to convert between them (e.g., I have a string, return the type to me). Since we are using interfaces to describe the resources, I don't know if that would be useful or not.
@Joshua Kelly @Ilya Beda @Josh Mandel @Brian Kaney
Joshua Kelly (Jul 23 2021 at 15:05):
@Gino Canessa I like the PR and adding the union type, I think both are useful.
Gino Canessa said:
I have not done performance testing on this yet. It would be trivial to add both options if that is easier for someone to use and compare.
I have a codebase where I am already using an enum that I can test both with (that's what provoked me to add to the thread).
A couple comments about the blog post. One of the examples in terms of differences between the two is outdated, extending for example: https://github.com/microsoft/TypeScript/pull/40998
Also:
Enums are compiled to some obscure code, instead of being just removed by the TypeScript compiler.
Personally I don't think it's obscure. Even if you compile to ES5 it's just an object with string values - Record<string, string>
- which is why you can enumerate it in the runtime.
I have a codebase which uses both @types/fhir and has it's own versions of both the union type and enum, I have a few examples (like the string template literal types case I originally posted) where I have it working with all of the intellisense stuff in VS Code. I'll volunteer to do some testing with it, any ideas on what to prove/test out here? @Gino Canessa
Brian Kaney (Jul 25 2021 at 21:55):
I was preparing for an update to DT with the readonly resourceType
and the work on generic support for Bundle. But I noticed this commit was merged recently: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/54356 (and see this: https://github.com/microsoft/dtslint/pull/335)
I don't want to undo what they did. @Gino Canessa should we put this upstream in FHIR codegen? Or should I add this when packing for DT (e.g. this is the packing project I use: https://github.com/Vermonster/fhir-dts-generator)
Brian Kaney (Jul 25 2021 at 21:56):
@Joshua Kelly
I'll volunteer to do some testing with it, any ideas on what to prove/test out here? Gino Canessa
For me, it really is editor performance. If there is no difference, I don't really have a strong opinion.
Gino Canessa (Jul 26 2021 at 16:44):
For the typing (adding undefined
to optional elements), that appears to be 'more correct', so I am putting it in now.
Related to editor performance, I've received a request to split up the definitions the way we do in some other languages (e.g., index.ts
+ patient.ts
, encounter.ts
, etc.). Thoughts? (@Brian Kaney , @Joshua Kelly , @Ilya Beda , @Josh Mandel )
Brian Kaney (Jul 26 2021 at 16:54):
@Gino Canessa how would splitting it up affect performance? Also if you did that, how would you configure your code to use, e.g. would use triple-slash/// <reference path="types/fhir/patient.d.ts" />
on each file were you would need it? I think for the magic of ts configured for DT, we would still have the entire namespace automatically be available (in @types/fhir
), right?
Gino Canessa (Jul 26 2021 at 18:31):
Many editors don't like massive files. Looking at the definition of Patient
means opening a file of about 150 lines instead of one that's over 400,000 lines.
We would want to setup a main file that includes everything else (e.g., r4.d.ts
). Consumers can still use a single include, but it's a bit easier to navigate. We'll want to sort out what the exact structure looks like to ensure there are no conflicts between the versions.
Joshua Kelly (Jul 27 2021 at 01:38):
The TS project-agnostic default should be a concatenated export of types but FHIR is a schema that can break defaults.
We would want to setup a main file that includes everything else
The most critical thing for a consumer, IMHO, is just to be able to access the entire namespace without having to explicitly import. That's what will be possible with r4.d.t.s
for ex, so I think this makes sense. The split examples are just an alt output.
Brian Postlethwaite (Jul 27 2021 at 05:46):
How does that handle extensions and contained parts? (as any?)
Brian Kaney (Jul 27 2021 at 12:37):
@Brian Postlethwaite -- extensions are formally defined, you can search for :
/**
* Optional Extension Element - found in all resources.
*/
export interface Extension extends Element {
And for contained is current set to:
contained?: FhirResource[] | undefined;
Where FhirResource is all the defined resource types.
Brian Postlethwaite (Jul 27 2021 at 12:43):
Isn't that with the full R4.d.ts file, was more referring to patient.d.ts
Brian Postlethwaite (Jul 27 2021 at 12:43):
Or does that format them drag in the other resources, which I'd have though eventually gets them all (due to contained)
Brian Kaney (Jul 27 2021 at 12:49):
Ah, I see what you mean. Yes I think patient.d.ts would have to reference all others because of contained.
Gino Canessa (Jul 27 2021 at 20:05):
Yep, that's part of the question - how would we want to structure files/modules so that this works and makes sense. I have it on my list, but was hoping that someone here had gone through the exercise already =)
Preston Lee (Jul 29 2021 at 20:28):
Brian Kaney said:
I was preparing for an update to DT with the readonly
resourceType
and the work on generic support for Bundle.
I would request that use of readonly
be reconsidered for only cases where no implementor could realistically need to override it in a subtype class.
Brian Kaney (Jul 29 2021 at 21:22):
@Preston Lee I think it is okay, if you mean this sort of thing -- https://bit.ly/3l9OB1P
Preston Lee (Jul 29 2021 at 22:20):
@Brian Kaney Doesn't that break though if you try let t: MyParent = aChild;
? "Types of property 'resourceType' are incompatible."
Brian Kaney (Jul 29 2021 at 22:54):
@Preston Lee - Yes, the type system would guard against that. Maybe I am not understanding your use-case if this is a problem for you. Could you make an example of your concern?
Ilya Beda (Aug 31 2021 at 13:17):
Hi!
I have one more thought.
Currently Reference is a regular type with no restrictions, so Reference.type
is an optional string.
What do you think about making Reference generic?
interface Reference<ResourceType = FhirResourceType > {
...
type: ResourceType;
...
}
It will be the first step to type-safe Reference type.
Once https://github.com/microsoft/TypeScript/issues/41160 get approved we can validate Reference.reference
to match ${ResourceType}/\s+
.
I would like to hear your thoughts.
Grahame Grieve (Aug 31 2021 at 13:21):
it's tricky in principle. you don't always know what the reference is, and you don't always care to look it up. and it's supposed to be the right type... but you might not want an error in an external system to cause an error your types
Ilya Beda (Aug 31 2021 at 13:30):
If an external system provides a reference with an incorrect type it most likely cracks runtime anyway when you try to handle it. If you are not handling it you are fine. Since TypeScript works only at the compile-time and doesn't cause any runtime errors because of types.
Gino Canessa (Aug 31 2021 at 21:40):
We have an open discussion on GitHub for this. If it makes sense, Reference
becomes a generic that can have a FHIR Type, or undefined
.
Ilya Beda (Sep 01 2021 at 08:27):
I remember that I saw the conversation somewhere, but can't find it. Thank you for the link!
Brian Kaney (Oct 04 2021 at 19:15):
Hey @Gino Canessa would it be a good time to refresh DT with the latest fhir-codegen?
Gino Canessa (Oct 15 2021 at 15:37):
Hi @Brian Kaney , apologies for the long delay in response. I'm happy to merge in current changes, but did have a policy question for the group.
I have a pass of the generator that splits up the type definitions into a .d.ts
file and the value-sets, code enumerations, etc. into a .ts
one, but the question becomes: what do we want to do with them?
My inclination would be to start pushing up the combination as an NPM package, but I wanted to see what everyone's thoughts were: yes, no, out of scope, etc.
I have been hoping to sketch out a generic TS/JS client package (similar to the .NET SDK from Firely or the HAPI SDK from SmileCDR). But unfortunately it's been too far down my list to get to it. Thoughts?
Brian Kaney (Oct 15 2021 at 21:11):
Hey @Gino Canessa -- Were you thinking this as a replacement or alternative or in addition to DT @types/fhir?
Gino Canessa (Oct 15 2021 at 21:31):
@Brian Kaney :shrug:. We can get most of what is needed with just DT. The biggest issue I ran into is the inability to express ValueSet and CodeableConcept instances, since those aren't type information. Use-case wise, I needed them so that I could list options in a drop-down for user selection.
For me personally, it's just as easy to run the codegen locally when I need something like that =). But I don't love that story, so I was asking what works best for everyone else.
Brian Kaney (Oct 15 2021 at 21:34):
I see. I kinda like the idea having @types/fhir and then a complemental npm library. I also find I build type guards all the time, so to have a nice NPM package that we could put that in would be really nice!
Brian Postlethwaite (Oct 16 2021 at 08:20):
What's the issue with CodeableConcepts? Just getting the code values not as basic strings?
I use the dt types quite successfully for expansions in questionnaire dropdowns.
Gino Canessa (Oct 18 2021 at 18:46):
The issue is that you cannot create an instance of an object in an interface file. So, we could make things like enums of the codes, but have no way of expressing the full instance (e.g., system, code, text, etc.).
Brian Postlethwaite (Dec 06 2021 at 04:26):
@Gino Canessa (or others) have you considered anything along the lines of the dotnet extension methods, except implemented in typescript?
I'm working on some front end stuff to try implementing this (alongside your definitioned typed fhir types)
Gino Canessa (Dec 06 2021 at 15:30):
@Brian Postlethwaite yeah, I have a 'want' item on my to-do list of a large TS library project.. but haven't been able to get to it. Right now, the code-gen output includes a lot of stuff that gets stripped for the Definitely Typed version (since it's a .d.ts); e.g., the R4 file in generated.
For my 'what do I want the end result to look like' tinkering, I installed the DT version and am keeping a file with additions I pull from the R4 generated output. I'm hoping that I can use that to inform the next iteration of output.
Brian Postlethwaite (Dec 06 2021 at 23:13):
Something you can share? I might be able to also contribute to the thoughts...
Brian Postlethwaite (Dec 06 2021 at 23:14):
(I'm working on SDC in typescript - now that my dotnet side is mostly cooking, and thought I'd do the same...)
Last updated: Apr 12 2022 at 19:14 UTC