refactor(): use TransferSyntaxUID constants from /constants/dicom.js …

[kursawero]

…in DicomMessage.js, Tag.js, data.test.js

The /constants/dicom.js was introduced recently (link to the pull request).

This pull request introduces a refactoring where some classes use the constants from this file to have the definition in a central place. In addition, the linter fixed some small formating issues automatically. I hope the linter settings are correct for this project.

[pieper]

I like the idea, but it looks like there are now circular dependencies and some other issues.

https://app.netlify.com/sites/dcmjs/deploys/6440d831f5cbdf00084671ac

[kursawero]

I will take a look.

[kursawero]

It looks like using "require" instead of "import ...." solved the issue with the constants, but this could be also just a simple configuration flag of the project that needs to be activated. I will look deeper.

I think the circular dependencies are historic, since we did not really changed the files. For me, it looks like the warning about that will not make the job failing and nobody spotted it before.

[pieper]

Yes, there were problems with the checks that meant they hadn't been run for a while so some issues may have existed already.

If that's the case then it would probably be okay to merge but it would also be nice if someone could track down the underlying cause.

[kursawero]

It seems that the import statement for the classes Tag.js and DicomMessage.js failed, because the rollup plugin is deprecated ( see rollupjs troubleshooting.).

There is also a message when you run the build process:

npm WARN deprecated rollup-plugin-babel@4.3.3: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-babel. npm WARN deprecated rollup-plugin-commonjs@9.3.4: This package has been deprecated and is no longer maintained. Please use @rollup/plugin-commonjs.

It seems that rollup needs to be upgraded too which seems to be not trivial, because you would jump from 1.31.1 to 3.21.0 if you bump to the latest version.

Do you have someone with rollup exprtise in the project? We are using Webpack and usually people with expertise are way faster in adapting the configurations.

[pieper]

Thanks for investigating. This project is fully community driven so if someone can step in and help with the upgrade that would be appreciated. Let's see if anyone else who uses the package has ideas.

[IbrahimCSAE]

I believe It's happening because of using module.exports, then trying to import using ES6 syntax in Tag.js. To fix this issue, I believe we can change the export style in dicom.js to ES6.

export const IMPLICIT_LITTLE_ENDIAN = "1.2.840.10008.1.2";
export const EXPLICIT_LITTLE_ENDIAN = "1.2.840.10008.1.2.1";
export const DEFLATED_EXPLICIT_LITTLE_ENDIAN = "1.2.840.10008.1.2.1.99";
export const EXPLICIT_BIG_ENDIAN = "1.2.840.10008.1.2.2";

export const UNDEFINED_LENGTH = 0xffffffff;
export const ITEM_DELIMITATION_LENGTH = 0x00000000;
export const SEQUENCE_DELIMITATION_VALUE = 0x00000000;

https://github.com/dcmjs-org/dcmjs/blob/4f1e6754e856d1aca9863def4b82b399bc0c8bb2/src/constants/dicom.js#L1

This will make the error message from rollup go away, I tried building locally, and it worked. You can use import, and you won't need to use require. So you would be able to revert the last commit, and It should build fine.

You can try this and see if it works now. I also made a pull request if that's easier.

[kursawero]

I adapted the pull request. Thanks for the help.

[IbrahimCSAE]

Of course!

[github-actions[bot]]

:tada: This PR is included in version 0.29.7 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket: