fix: Allow the mode setup/creation to be async, and provide a few more values to extension/app config/mode setup. #4016
Conversation
✅ Deploy Preview for ohif-platform-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for ohif-dev canceled.
|
| @@ -40,7 +40,7 @@ async function appInit(appConfigOrFunc, defaultExtensions, defaultModes) { | |||
|
|
|||
| const appConfig = { | |||
| ...(typeof appConfigOrFunc === 'function' | |||
| ? await appConfigOrFunc({ servicesManager }) | |||
| ? await appConfigOrFunc({ servicesManager, loadModules }) | |||
There was a problem hiding this comment.
The load modules is added here so that the app config can use modules listed in the loadModules.
| @@ -111,7 +111,7 @@ async function appInit(appConfigOrFunc, defaultExtensions, defaultModes) { | |||
| ? appConfig.modesConfiguration[id] | |||
| : {}; | |||
|
|
|||
| mode = mode.modeFactory({ modeConfiguration }); | |||
| mode = await mode.modeFactory({ modeConfiguration, loadModules }); | |||
There was a problem hiding this comment.
And the modeFactory is made async capable so that it can wait for files to load/modules internal to load.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4016 +/- ##
=======================================
Coverage 44.37% 44.37%
=======================================
Files 80 80
Lines 1334 1334
Branches 327 327
=======================================
Hits 592 592
Misses 589 589
Partials 153 153 ☔ View full report in Codecov by Sentry. |
Passing run #4028 ↗︎
Details:
Review all test suite changes for PR #4016 ↗︎ |
|||||||||||||||
|
I understand your intention. I asked Ibrahim to initiate a similar effort for dev dependencies like percy, cypress, docusaurus, etc. Here is his PR: https://github.com/OHIF/Viewers/pull/4188/files I prefer his approach. What are your thoughts, @wayfarer3130? |
| @@ -57,6 +57,6 @@ | |||
| "eslint-loader": "^2.0.0", | |||
| "webpack": "^5.50.0", | |||
| "webpack-merge": "^5.7.3", | |||
| "webpack-cli": "^4.7.2" | |||
| "webpack-cli": "^5.0.2" | |||
There was a problem hiding this comment.
i think we are using another version elsewhere
There was a problem hiding this comment.
Updating that overall, so leaving this one.
Context
Adds the extension/mode loader to the app config setup so that things listed in the loader can be provided/used.
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment