-
Notifications
You must be signed in to change notification settings - Fork 230
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
why dicomParser does not have pako as dependency #270
Comments
This is a little before my time (way back in 2015), but my guess would be so that users who don't need inflation don't have to take the hit of the extra bytes from bundling pako. It's also not needed for running in node.js, which has the native zlib library, and there's no separate build pipeline at the moment for browser vs. node. @chafey would know for sure |
@yagni is correct, paki is optional to avoid adding a dependency.
Dependency management was much more difficult back then
…On Thu, Jun 13, 2024 at 5:25 PM yagni ***@***.***> wrote:
This is a little before my time (way back in 2015), but my guess would be
so that users who don't need inflation don't have to take the hit of the
extra bytes from bundling pako. It's also not needed for running in
node.js, which has the native zlib library, and there's no separate build
pipeline at the moment for browser vs. node. @chafey
<https://github.com/chafey> would know for sure
—
Reply to this email directly, view it on GitHub
<#270 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJVXWVEUDTW5C5UWQHROXLZHIL6FAVCNFSM6AAAAABJJGKBZ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRWHA4TGMJSGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I understand, but I'm encountering issues with ESM for the DICOM image loader, and assuming a library is available on the |
Adding support for injecting it via an init function makes sense. Just
don't make it mandatory as dicomParser is widely used as it is today
…On Fri, Jun 14, 2024 at 8:00 AM Alireza ***@***.***> wrote:
I understand, but I'm encountering issues with ESM for the DICOM image
loader, and assuming a library is available on the window object is an
anti-pattern, in my opinion. At the very least, it should be injected
during initialization or a similar process.
—
Reply to this email directly, view it on GitHub
<#270 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJVXWRSI5IEE67KKW2MT6DZHLSQNAVCNFSM6AAAAABJJGKBZ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNRXHE4DOOJQHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
I'm trying to build an ES module for
dicom-image-loader
and encountering several issues. It seems likedicom-parser
attempts to check ifpako
is defined or not, and then decides whether to use it. My question is, why not simply depend onpako
directly? Currently,dicom-parser
has zero dependencies and only dev dependencies.@yagni
The text was updated successfully, but these errors were encountered: