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

Integrate FPL v2 #195

Merged
merged 13 commits into from
Feb 6, 2025
Merged

Integrate FPL v2 #195

merged 13 commits into from
Feb 6, 2025

Conversation

jafeltra
Copy link
Collaborator

@jafeltra jafeltra commented Dec 31, 2024

Note: This PR is in marked as a draft because the FSHHelpers.test.jss suite does not run yet but the general approach to integrating FPL v2 and new SUSHI and GoFSH is stable. Jan 7 update: this is ready for review now.

Description:
This PR updates FSH Online to use the new FPL v2 and newer versions of SUSHI and GoFSH that use FPL v2 as well.

This PR cleans up a good amount of code that was moved into FPL. I also tried to use some newer SUSHI functions where possible, rather than having FSH Online write its own versions.

The way I had to create to make some temporary definitions for GoFSH is a little odd, but it was the only way I could figure out how to use a FHIRProcessor to processConfig to get all the dependencies from an IG resource, which we needed to properly initialize the BrowserBasedPackageCache.

Note the following changes still need to be made to this branch before it can be merged:

  • The FSHHelpers.test.js file cannot load the sql-wasm.wasm file correctly right now, so that needs to be figured out. The assetsInclude configuration field is what makes this work when running a development or production server, so I guess the testing equivalent needs to be figured out.
  • Once the FSHHelpers.test.js file properly loads, the tests need to be updated to mock out the pieces that FPL is doing now, because it still is mocking out parts from the old FSH Online Processing functions, which do not exist anymore.
  • The correct SUSHI and GoFSH versions need to be used in the package.json once they're available and the versions need to be updated in the TopBar component.

Jan 7 update: these have all been done.

Testing Instructions:
Ttry using this locally in your browser to make sure there isn't functionality that I've missed that needs to be added in FPL or here. You should be able to:

  • download and cache packages in your browser's IndexedDB -- you can clear your IndexedDB to ensure new packages are being downloaded, but also the default name of the database has changed, so the first time you run this, it should try to download and cache the package
  • load packages from the cache if they're already downloaded
  • run SUSHI without configured dependencies and with one or more configured dependencies
  • run GoFSH without configured dependencies, with one or more configured dependencies, and with dependencies that are only specified in an ImplementationGuide resources

Related Issue:
N/A

- Removes downloading and loading definitions into IndexedDB and
  getting resources from IndexedDB for FHIRDefinitions -- moved to FPL
- Adds new FPL, new SUSHI, and new GoFSH

Note: FSHHelper tests are still broken
@jafeltra jafeltra marked this pull request as ready for review January 7, 2025 17:38
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

I'm so glad this is working! Thank you! I left two comments about possible places we could extract functions to make the code a little cleaner. Let me know what you think!

src/utils/FSHHelpers.js Outdated Show resolved Hide resolved
src/utils/FSHHelpers.js Outdated Show resolved Hide resolved
@jafeltra
Copy link
Collaborator Author

@cmoesel - 65adeb5 pulled out the code for loading the FHIRDefinitions into a separate function and calls that function for SUSHI and GoFSH. I did have to add a little bit of specialized logic because the configuration that the GoFSH loadExternalDependencies uses is a different type than the configuration than the SUSHI loadExternalDependencies. Let me know what you think. If you don't like it, it's fine by me if you revert it. That commit also pulls out the code that had to set up temp variables just to determine the GoFSH configuration into a separate function, which kind of hides its weirdness.

One thing I just noticed is that there's an error when GoFSH is run before SUSHI. This is because GoFSH is setting up a LakeOfFhir:

const lake = new processor.LakeOfFHIR(docs);
await lake.prepareDefs();

However, the FHIRDefinitions that LakeOfFHIR uses at that point is uninitialized, so it tries to initialize it, but the FHIRDefinitions is made with the default packageDB and there's no way to provide the needed override to use the locateFile option. I didn't have a chance to look too much into fixing it, but I think it might require an update to GoFSH, which is a little unfortunate. Maybe there's another way around it though. It probably is best to not merge this until that is fixed though.

@jafeltra
Copy link
Collaborator Author

jafeltra commented Feb 4, 2025

@cmoesel - I think this is ready for review!

65adeb5 - pulls out the code for loading the FHIRDefinitions into a separate function and calls that function for SUSHI and GoFSH. I did have to add a little bit of specialized logic because the configuration that the GoFSH loadExternalDependencies uses is a different type than the configuration than the SUSHI loadExternalDependencies. Let me know what you think. If you don't like it, it's fine by me if you revert it. That commit also pulls out the code that had to set up temp variables just to determine the GoFSH configuration into a separate function, which kind of hides its weirdness.

e0ace2a - initializes sql.js once for SUSHI and once for GoFSH so it is already properly initialized for any other places that use it. That means we don't need thelocateFile override, but that's okay.

1c217a9 - filters out any provided dependency that doesn't have a version. That makes it so if anyone has a typo in the configuration modal (like hl7.fhir.us.core@6.0.0 instead of hl7.fhir.us.core#6.0.0), FSH Online continues running and doesn't try to load that dependency, since it would break later down the line without a version.

Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

This looks good. I have one request. Let me know if it is non-trivial.

src/utils/helpers.js Outdated Show resolved Hide resolved
Copy link
Member

@cmoesel cmoesel left a comment

Choose a reason for hiding this comment

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

This looks good! Thank you, @jafeltra. You've now proven that the FPL browser-based implementations work -- and you've unstuck us from that prior version of SUSHI. Thanks!

@jafeltra jafeltra merged commit df04c28 into master Feb 6, 2025
4 checks passed
@jafeltra jafeltra deleted the new-fpl branch February 6, 2025 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants