cornerstone3D icon indicating copy to clipboard operation
cornerstone3D copied to clipboard

Cornerstone3D and Vite

Open ty-ler opened this issue 4 years ago • 6 comments

I have been attempting to utilize Cornerstone3D in a Svelte web app which uses Vite. Unfortunately, when I try to import @cornerstonejs/core into the app, the build fails.

image

image

Note: If I import things from @cornerstonejs/core/dist/umd/index everything compiles and works fine, but I cannot utilize the TS types.

Reproduction steps:

  1. Generate new Svelte app using Vite (npm create vite@latest)
  2. npm install
  3. npm install @cornerstonejs/core (etc...)
  4. npm run dev
  5. Above error is thrown

I've created a repo with nothing but a fresh Svelte app and Cornerstone3d to demonstrate the problem: https://github.com/ty-ler/cornerstone3d-svelte-vite-repro

ty-ler avatar Apr 02 '22 21:04 ty-ler

Thanks for the reproduction repo!

I was actually also wondering what we should be using for the main/module etc fields in package.json for Cornerstone. We currently output three things: UMD, CommonJS, and ESM.

My understanding is that we need UMD for browser-based script-tag usage, so I put that as the default main.

I was under the impression that ESM was not widely supported yet, but was the intended future format, so I put that as module and also pointed the types to the index.d.ts in the ESM folder.

I left CommonJS in there in case someone needed it for something.

We could potentially switch main to the ESM version and then add the browser property to point to UMD instead (https://docs.npmjs.com/cli/v8/configuring-npm/package-json#browser). You could try doing this, rebuilding, and then using yarn link to see if this resolves your issue. If it does, and there's no other downsides I'd be happy to merge it.

My only hesitation with switching to ESM as main is that we are currently using the ESM output generated by TypeScript, and it does not generate valid ESM modules, since they are missing the .js in their imports (see this thread https://github.com/microsoft/TypeScript/issues/40878). It doesn't look like the TypeScript team plans to fix this issue, so we would probably need to find a workaround for that. One super-ugly workaround that people describe is to use .js while importing from a .ts file - the .js won't get transpiled out and so the result will be valid. I hate the idea of doing that though but haven't had a chance to look for a better alternative.

swederik avatar Apr 03 '22 13:04 swederik

Hey, thanks for the response!

I tried out several things including changing browser to point to the UMD entry point and changing main to point to ESM, but nothing seemed to solve my issue.

Something I don't quite understand is why the module field uses the .ts extension vs .js. The output files in dist/esm don't actually include a .ts file anywhere (only .d.ts). I assume that some bundlers are able to work out the file name discrepancy using the associated source map file. Though, maybe there's something that I'm just not fully understanding.

I noticed that when I tried switching the module extension to .js, I received a different error:

✘ [ERROR] Could not resolve "lodash.clonedeep"

    node_modules/@cornerstonejs/core/dist/esm/volumeLoader.js:4:22:
      4 │ import cloneDeep from 'lodash.clonedeep';
        ╵                       ~~~~~~~~~~~~~~~~~~

I'm wondering if the lodash.clonedeep imports are changed to a different style of import - would we be able to use the .js extension for module, and not have to change the main or browser fields at all? Lodash offers a few different ways to import the library. When I am using lodash in my own projects, I usually import methods using the lodash/cloneDeep style.

I will do some testing on my own now and update if I make any progress. Just wanted to document my thoughts so far.

ty-ler avatar Apr 03 '22 20:04 ty-ler

So after some testing just now, I believe I have figured out what was causing my issue. It is unrelated to the part about lodash above. The error with lodash showed up after manually editing the package.json for @cornerstonejs/core from inside the node_modules of my Vite project.

What wasn't fully obvious to me was the effect that changing .ts to .js had at build time vs. just changing the extension in the (already built) installed module package.json.

When I changed the extension to .js, rebuilt @cornerstonejs/core, linked, then finally installed the linked package in my Vite project, everything seemed to work great.

I'll create a PR for this change in a few minutes.

ty-ler avatar Apr 03 '22 21:04 ty-ler

FYI I tried to get your repo working and ran into some other issues:

https://github.com/ty-ler/cornerstone3d-svelte-vite-repro/pull/1#issue-1191254076

Repeated here:

  • resemblejs should not be in dependencies (it pulls in node-canvas which we definitely do not want)
  • detect-gpu and @kitware/vtk.js should be documented as peer deps (or possibly we want to include them as dependencies?)
  • lodash.clonedeep should probably just be a dependency of core
  • cornerstone-wado-image-loader should be a dependency of the streaming volume loader
  • Something is wrong with the export of RenderingEngine. It still does not import correctly on this PR, and I don't see why not. It looks to be declared normally...

We'll try to iron these out ASAP. I'm not sure what is wrong with the export of RenderingEngine...

swederik avatar Apr 04 '22 05:04 swederik

Just to follow up, we tested a webpack setup and didn't have the same issue (RenderingEngine showing up as undefined) that I see in the vite example, so that leads me to conclude it's something wrong with vite or Svelte, and not an issue with the ESM build itself.

swederik avatar Apr 04 '22 09:04 swederik

In my case it was complaining that it was missing dependency "@kitware/vtk.js"

So I just did npm -i @kitware/vtk.js and got it to work.

huntedman avatar Jul 13 '22 11:07 huntedman