OHIF / Viewers

OHIF zero-footprint DICOM viewer and oncology specific Lesion Tracker, plus shared extension packages
https://docs.ohif.org/
MIT License
3.35k stars 3.36k forks source link

Server-Side Rendering (SSR) support in core project #951

Closed sedenardi closed 5 years ago

sedenardi commented 5 years ago

Request

What feature or change would you like to see made?

Support for server-side rendering in @ohif/core.

Why should we prioritize this feature?

It's preventing (the probably small amount of) people from using the project in SSR applications.

More

I'm currently exploring building a DICOM viewer to integrate with our existing web application. Our project uses React, and we want to fully customize the look+feel and behavior of the viewer, so both the standalone viewer and the script embedded versions won't cut it (however the /viewer project is proving to be extremely educational!).

We're starting to use the @ohif/core project to take care of DICOMWeb fetching (rather than use dicomweb-client directly). Our application utilizes server-side rendering for performance and SEO purposes, and when we render the page on our server we get the window is not defined error (common with packages that expect a browser environment).

Looking through the codebase, I see window referenced in

I also see navigator and document referenced in a few other files, which would produce similar problems.

Would there be any objections to me submitting a PR to address these areas? The fix would be different depending on the file being addressed, but the overall goal would be to allow the library to be imported on the server, and any actions that expect a browser environment and don't find one to "fail" gracefully.

sedenardi commented 5 years ago

Forgot to mention, this would also allow the project to be used on the server for non-rendering actions, such as using DICOMTagDescriptions for parsing DICOM files in conjunction with dicom-parser.

dannyrb commented 5 years ago

@sedenardi, those sound like very reasonable requests. More than happy to accept pull requests that move toward this goal. It may be worth adding a lint rule, or something similar, that would error when CI runs to make maintaining support easier.

Thanks for the kind words and for reaching out. Looking forward to working with you ^_^

sedenardi commented 5 years ago

Digging in a bit more, it seems like my initial assessment missed a few important things. Most notably, that @ohif/core's dependencies - cornerstone-core, cornerstone-tools, and cornerstone-wado-image-loader - understandably rely on browser globals. So simply reducing the at-import dependence on browser globals in core wouldn't really do much.

I'm going to do a little more digging and see if what I'm thinking is feasible. I don't want to disrupt a project in order to satisfy an edge case or a non-primary use case.

stale[bot] commented 5 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

dannyrb commented 5 years ago

👋 Hi @sedenardi, I'm going to close this issue as it's gone stale. If you or anyone else is interested in typing up a list of the required items / steps to accomplish this, we can add it to the backlog and use it as a roadmap for others (or yourself to report progress).

sedenardi commented 5 years ago

@dannyrb No problem, thank you.