cornerstone3D
cornerstone3D copied to clipboard
Try Remove Cicular Deps
fixes https://github.com/cornerstonejs/cornerstone3D/issues/742 Continuing the discussion started herer : https://github.com/cornerstonejs/cornerstone3D/issues/742
The problem i found in that many file import module using the root index, especially types.
So as the index indexes almost everything, the index import modules that imports types using the index creating cicular deps.
I tried to start removing them, but there's a lot of circular deps
What do you think, should i continue or is some reachitecturing import neede?
Deploy Preview for cornerstone-3d-docs ready!
| Name | Link |
|---|---|
| Latest commit | 2974a991a9db646199fd778e2cd0a326fba794e5 |
| Latest deploy log | https://app.netlify.com/sites/cornerstone-3d-docs/deploys/65162ba06a6a5b0008568973 |
| Deploy Preview | https://deploy-preview-798--cornerstone-3d-docs.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
so where this happens? during yarn install?
when building cornerstone in a project with vite there a lot of circular deps warning, and the tools are not working on the built project. I checked the circular deps with dpdm to list them
I tried to fix all the types with eslint, probably that is good enough for majority of the issues, though for some reason dpdm shows a lot more than madge (https://www.npmjs.com/package/madge) which seems to be a lot more used.
I added a script to packages called dependency-test so you can run it via yarn run dependency-test
After fixing the type imports via import type
- I fixed the two non-type circular dependency in core -> madge pass
- There are 5 circular dependency in tools that I haven't figure out a way to fix it, seems to be more complex and require some thinking for refactoring
1) stateManagement/annotation/annotationSelection.ts > stateManagement/annotation/annotationState.ts > stateManagement/annotation/FrameOfReferenceSpecificAnnotationManager.ts > stateManagement/annotation/annotationVisibility.ts
2) stateManagement/annotation/annotationState.ts > stateManagement/annotation/FrameOfReferenceSpecificAnnotationManager.ts > stateManagement/annotation/annotationVisibility.ts
3) stateManagement/segmentation/segmentationState.ts > stateManagement/segmentation/triggerSegmentationEvents.ts
4) store/index.ts > store/removeEnabledElement.ts > store/SynchronizerManager/getSynchronizersForViewport.ts
5) store/index.ts > store/removeEnabledElement.ts > store/ToolGroupManager/getToolGroupForViewport.ts
- streaming loader pass
- dicom image loader has 12 errors
1) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadors/index.ts > imageLoader/wadors/getPixelData.ts > imageLoader/internal/index.ts > imageLoader/internal/xhrRequest.ts
2) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadors/index.ts > imageLoader/wadors/loadImage.ts
3) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadors/index.ts > imageLoader/wadors/loadImage.ts > imageLoader/createImage.ts
4) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadors/index.ts > imageLoader/wadors/loadImage.ts > imageLoader/createImage.ts > imageLoader/convertColorSpace.ts > imageLoader/colorSpaceConverters/index.ts > imageLoader/colorSpaceConverters/convertPALETTECOLOR.ts
5) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadors/index.ts > imageLoader/wadors/loadImage.ts > imageLoader/createImage.ts > imageLoader/getImageFrame.ts
6) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadors/index.ts > imageLoader/wadors/metaData/index.ts > imageLoader/wadors/metaData/metaDataProvider.ts
7) imageLoader/wadors/metaDataManager.ts > imageLoader/wadors/retrieveMultiframeMetadata.ts
8) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadouri/index.ts > imageLoader/wadouri/dataSetCacheManager.ts
9) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadouri/index.ts > imageLoader/wadouri/dataSetCacheManager.ts > imageLoader/wadouri/dataset-from-partial-content.ts
10) imageLoader/wadouri/dataSetCacheManager.ts > imageLoader/wadouri/retrieveMultiframeDataset.ts
11) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadouri/index.ts > imageLoader/wadouri/getEncapsulatedImageFrame.ts
12) externalModules.ts > imageLoader/registerLoaders.ts > imageLoader/wadouri/index.ts > imageLoader/wadouri/metaData/index.ts > imageLoader/wadouri/metaData/metaDataProvider.ts
- adapters looks fine
- nifti looks fine
Do you have any one that can work on these?
hummm i see the problem, for instance the xrfRequest should better event the initiation and the result of the request to a listener taht will have cornerstone reference rather then trying to get it's own cornerstone reference which create a circular dependency... The refactoring is going to be not that simple, for now celian is in school for 6w, will try to see.
In any case, this is a great first step to find out what is wrong, though i'm not sure why results of dpdm and madge are different
For the dicomimageloader ones there certainly need to be breaking changes, so can we merge this into beta?
There are only 4 circ deps left report by madge
Great !
Yeah I think you can merge this, it already reduce the problem and also prevent other commits to add more circular deps
When Célian comes back (November) he can look at the remaining circular deps
@salimkanoun I want to change this base to beta, since it is a braking change, we ship it in 2.0
Sounds good
I'll close this and do it myself in the beta since it is outdated, thanks