Use new FPL API #277
Use new FPL API #277
Conversation
The FPL version 2 package includes API changes. Update classes, functions, and tests that include loaded dependencies to use the new API. Most of this is acquired through SUSHI's FHIRDefinitions class, which extends FPL's BasePackageLoader class. The main thing to watch out for is that FHIRDefinitions must be initialized before use, which is an async operation. The LakeOfFHIR class is updated to have an instance of FHIRDefinitions for access to its fishing methods. Therefore, it also needs to be initialized asynchronously. There may be some possible redesigns of the LakeOfFHIR class in order to more fully make use of the new FPL API.
There was a problem hiding this comment.
This looks good, and thankfully doesn't seem to have affected too many things. I've left a few comments for your consideration. I've also noticed that when I run the tests, I'm now getting a lot of log messages about loading a virtual package. I think the spy used to suppress those but it seems it is not anymore. Have you looked into it?
src/utils/MasterFisher.ts
Outdated
| public lakeOfFHIR = new LakeOfFHIR([]), | ||
| public external = new FHIRDefinitions() | ||
| public lakeOfFHIR: LakeOfFHIR, | ||
| public external: FHIRDefinitions |
There was a problem hiding this comment.
I noticed that you guard external below (e.g., this.external?.fishForFHIR(item, ...types)) -- so you should probably mark it optional here (e.g., public external?: FHIRDefinitions).
test/optimizer/plugins/ResolveValueSetComponentRuleURLsOptimizer.test.ts
Outdated
Show resolved
Hide resolved
For LakeOfFHIR, since assignMissingIds and removeDuplicateDefinitions are preparatory steps, add them to the beginning of prepareDefs. Since they should only be used as these preparatory steps, the methods are also marked as private. FSH grammar supports designation on a concept in a CodeSystem. These test fixtures are therefore now supported and renamed accordingly. This means that since only a missing name and id makes a CodeSystem unprocessable, and an id will be added by LakeOfFHIR if it is missing, it means that any CodeSystem resource is processable after LakeOfFHIR.prepareDefs is called. Add helper function to simplify test setup in ResolveValueSetComponentRuleURLsOptimizer. Mark FHIRProcessor tests as async, since the calls to restokeLake should be awaited. Minor cleanup in some other files.
|
I've made updates based on review comments from @cmoesel , and since the published SUSHI and FPL dependencies are now available, I've marked this PR as ready for review. |
| const defs = new FHIRDefinitions(); | ||
| await defs.initialize(); |
There was a problem hiding this comment.
Does it make sense to make a createFHIRDefinitions function like in SUSHI?
src/processor/LakeOfFHIR.ts
Outdated
| // The simplest approach is just to re-use the FHIRDefinitions fisher. But since this.docs can be modified by anyone at any time | ||
| // the only safe way to do this is by rebuilding a FHIRDefinitions object each time we need it. If this becomes a performance | ||
| // concern, we can optimize it later -- but performance isn't a huge concern in GoFSH. Note also that this approach may need to be | ||
| // updated if we ever need to support fishing for Instances. | ||
| const defs = new FHIRDefinitions(); | ||
| this.docs.forEach(d => defs.add(d.content)); | ||
| return defs.fishForFHIR(item, ...types); | ||
| return this.defs.fishForFHIR(item, ...types); | ||
| } |
There was a problem hiding this comment.
Is this comment still applicable since we're not re-building a FHIRDefinitions object each time anymore?
| const defs = new FHIRDefinitions(); | ||
| this.docs.forEach(d => defs.add(d.content)); | ||
| return defs.fishForMetadata(item, ...types); | ||
| return this.defs.fishForMetadata(item, ...types); | ||
| } |
There was a problem hiding this comment.
Similar question about the comment here as above.
Remove unneeded comments from LakeOfFHIR.
Description:
The FPL version 2 package includes API changes. Update classes, functions, and tests that include loaded dependencies to use the new API. Most of this is acquired through SUSHI's FHIRDefinitions class, which extends FPL's BasePackageLoader class. The main thing to watch out for is that FHIRDefinitions must be initialized before use, which is an async operation.
The LakeOfFHIR class is updated to have an instance of FHIRDefinitions for access to its fishing methods. Therefore, it also needs to be initialized asynchronously. There may be some possible redesigns of the LakeOfFHIR class in order to more fully make use of the new FPL API.
Testing Instructions:
This currently needs thenew-fplbranch of SUSHI as a dependency. Until this branch is part of a SUSHI release, you'll need to make that dependency yourself. Pull down thenew-fplbranch of SUSHI, build it,npm packit into a.tgzfile, then use that as the dependency. Until the dependency is available, this PR will be a draft.Update 2024/12/24: the updates to SUSHI and FPL are now available in published releases. The dependencies in this PR are updated accordingly.
There are a lot of changes to the tests due to the FPL API changes. There should be no changes in output FSH resulting from this change, so try it out with some FHIR resources and make sure the FSH you get is the same before and after the changes.
Related Issue:
N/A