Code Modernization

[jens-ox]

This should probably rather be a project than an issue 😅

Originally, I wanted to contribute to NiiVue by either adding TypeScript declarations or directly porting NiiVue to TypeScript. Moving to TS is a larger undertaking, so it makes sense to do it incrementally. This issue lists some steps than can be done before, during and after a migration.

Testing Setup

Currently, Jest and Puppeteer are used for testing, with Puppeteer probably getting replaced by Playwright in the future (#744).

Additionally, I would recommend using Vitest instead of Jest. This aligns with the existing usage of Vite across the repo, reduces the amount of dependencies and should make the tests faster. It will also make testing TypeScript code easier down the road.

It might also make sense to add unit tests for complex methods - this doesn't have to be done before a TS migration, though.

Code Quality

Moving to TypeScript will expose inconsistencies in the codebase - reducing them beforehand makes the migration easier. For that, the following things can be done:

• Set up strict linting rules (#743)
• Use modern JavaScript language features (e.g. #745)

Building/Bundling

Currently, NiiVue exposes one UMD and one ES bundle, which both include all NiiVue dependencies. While this is great for direct in-browser usage via e.g. Unpkg, using NiiVue by installing it via NPM will cause the application using NiiVue to include the whole minified bundle instead of being able to minify NiiVue itself. As a consequence, multiple instances of e.g. rxjs might be included if the application also depends on rxjs.

My recommendation would be to remove the UMD build and only expose NiiVue as ES Module if possible. This would mean that the current build step could be more or less completely removed.

As a reference, this is what the current build bundle looks like:

TypeScript migration and follow-ups

Once those three steps are done, a TypeScript migration should be relatively straight-forward.

See also #769 for a list of sensible follow-ups.

[hanayik]

Amazing insight and summary of the work to be done. Thanks a lot @jens-ox. I'll go through some of this today and add comments. You've been a great help over the past few days. Really impressive that you have just jumped right into the NiiVue code base and been so productive :).

[jens-ox]

As a sidenote - I also opened up two PRs in NiiVue dependencies:

• https://github.com/rii-mango/Daikon/pull/57
• https://github.com/rii-mango/JPEGLosslessDecoderJS/pull/17

[hanayik]

Yes, I saw this. Thanks a lot for helping modernize those repos too. I’ve been wanting to update the build configs for these (and nifti-reader-js) but have not had the time to do so yet. Perhaps this is a strategic objective we can plan and prioritise in upcoming activities. Would be good to discuss at the next NiiVue group meeting.

From: Jens Ochsenmeier @.> Date: Wednesday, 22 November 2023 at 15:19 To: niivue/niivue @.> Cc: Taylor Hanayik @.>, Comment @.> Subject: Re: [niivue/niivue] Code Modernization (Issue #746)

As a sidenote - I also opened up two PRs in NiiVue dependencies:

• rii-mango/Daikon#57https://github.com/rii-mango/Daikon/pull/57
• rii-mango/JPEGLosslessDecoderJS#17https://github.com/rii-mango/JPEGLosslessDecoderJS/pull/17

— Reply to this email directly, view it on GitHubhttps://github.com/niivue/niivue/issues/746#issuecomment-1822971624, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABHZ3OHSHBUQ2RFG4GNVCJLYFYJXJAVCNFSM6AAAAAA7UKAT3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMRSHE3TCNRSGQ. You are receiving this because you commented.Message ID: @.***>

[neurolabusc]

Related to comments for Daikon PR57, I do wonder if we should include Daikon directly in the NiiVue build - DICOM is a moving target, this is a huge dependency, and Daikon has not seen much recent work despite most manufacturers moving to enhanced DICOM. I do also think WASM and the actively developed dcm2niix may provide a better long term solution than Daikon.

So I guess my own preference is we consider some way to use Daikon as a plugin but do not include it as a core feature of NiiVue.

[hanayik]

Dropping DICOM would be a significant feature change to NiiVue, so we should discuss it at an upcoming group meeting and post it as an issue in the NiiVue repo for the community to respond to. I do think most users in our field won’t rely on DICOM, but some may have come to depend on it for other applications.

I’m also not sure what the best method is to have it as a plugin (or optional feature). So would be good to investigate that.

[jens-ox]

@hanayik

Perhaps this is a strategic objective we can plan and prioritise in upcoming activities. Would be good to discuss at the next NiiVue group meeting.

Sounds good. I think that with https://github.com/rii-mango/JPEGLosslessDecoderJS/pull/17, JPEGLosslessDecoderJS is now in the state that I would consider to be ideal - modern language standards, everything written in TypeScript, tests via vitest, modern build toolchain (esbuild via tsup). Having that setup for Daikon, NiiVue and friends would be the goal imo.

Dropping DICOM would be a significant feature change to NiiVue

I agree - I want to use NiiVue for a project that heavily relies on DICOM files. As long as there's no urgent reason to drop support for DICOM, I would recommend keeping it. That allows a more diverse usage of NiiVue.

[jens-ox]

Just in case @rii-mango is unresponsive:

One option would also be to separately publish Daikon and JpegLosslessDecoderJs as separate packages (e.g. @niivue/daikon and @niivue/jpeg-ls-decoder). The code could be hosted in this repository as a monorepo setup, which would also make sense in the long run in order to separate the NiiVue library from the demo code.

[hanayik]

@jens-ox , I think we have addressed these suggestions over the past few weeks (testing setup, code quality, and building). Your commits to modernise niivue have been great. I'm in favour of closing this issue and starting new bite-sized issues for anything remaining that could be related.

The other projects (that niivue depends on) have their own issues started, so discussions can happen in those threads as needed.