fix(rollup): Do not bundle vtk.js

[floryst]

In reference to #28, we should not bundle @kitware/vtk.js. Additionally, this directly impacts vtkMapper, as setting global/static coincident topology parameters is done at a module level, and bundling vtk.js with react-vtk-js means it's impossible to set global parameters for react-vtk-js.

This will mean all consumers must install @kitware/vtk.js alongside react-vtk-js. Should this be a breaking change, or are we okay having people realize this once npm/yarn complains about missing peer deps?

[jourdain]

Hum, I think we need to talk about it... I originally made that lib so it can be standalone with the proper tree-shaking. Also your comment is not fully true as you can get your hand on the vtk.js that is bundled inside the react-vtk-js to set the coincident topology parameters.

[floryst]

I'm not sure how to @kitware/vtk.js/Rendering/Core/Mapper through react-vtk-js, unless it's something like react-vtk-js/dist/esm/node_modules/@kitware/vtk.js/.... That's doable, but kind of messy.

[jourdain]

It is more about getting a ref on GeometryRepresentation.mapper.

[floryst]

I'm not sure how a ref on GeometryRepresentation will give me the internally used vtkMapper module. I can see how it will expose the mapper instance, but some of the coincident topology parameters are on the mapper module itself.

[jourdain]

Are the static methods on both, the class and instance? If just class, then yes I agree with you.

I mean if we change that behavior, we will need to fix dash-vtk.

[floryst]

After some discussion, I think the move forward is to have the ESM build truly use vtk.js as a peer dep (vtk.js as external). The CJS and UMD builds can still bundle vtk.js.

[agirault]

Is this resolved?

[floryst]

Nope, not yet. AFAIK it doesn't block anything, but it's a nice-to-have.

[jourdain]

This should be taking care thanks to #41