Fix typed arrays serialization deserialization

[bourdaisj]

Context

The behavior of JSON.stringify for typed arrays is to serialize them as objects with numeric keys. This cause crashes when re-building the object since it would try to fit an object where a typed array is required. This changes makes it so getState replaces typed arrays with plain javascript arrays.

Typical example is vtkImageData direction property, which is by default Float64Array
minimal reproduction:

import vtk from '@kitware/vtk.js/vtk';
import vtkImageData from '../../Sources/Common/DataModel/ImageData';

const imdt = vtkImageData.newInstance();
const jsonRepr = JSON.stringify(imdt);
const jsonDeserialize = JSON.parse(jsonRepr);

const vtkObj = vtk(jsonDeserialize); // this should crash

see also Kitware/glance#480

Results

makes serialization/deserialization possible and successful

Changes

• Documentation and TypeScript definitions were updated to match those changes

This change makes it so we serialize typed arrays as arrays.

PR and Code Checklist

• semantic-release commit messages
• Run npm run reformat to have correctly formatted code

Testing

• This change adds or fixes unit tests
• Tested environment:
  • vtk.js: master
  • OS: linux
  • Browser: chrome, firefox latest

[bourdaisj]

fyi @finetjul @bruyeret

[bruyeret]

LGTM 👍

[finetjul]

I can think of 2 better (not orthogonal) alternatives:

• vtkImageData.indexToWorld and worldToIndex must not be typed array but regular arrays, which are 64 bits anyway.
• vtkImageData.indexToWorld and worldToIndex shall not be serialized but must be flagged as protected by adding _ prefix and recomputed at deserialization.

[bourdaisj]

to me those are not alternatives to the fix I'm proposing which is necessary but definitely you have a good point.
probably gonna include those in the scope of this PR

[finetjul]

to me those are not alternatives to the fix I'm proposing which is necessary but definitely you have a good point.
probably gonna include those in the scope of this PR

I agree, your fix is also necessary.
I have that bad habit to not willing to fix a crash if it can help detect bad design in the first place.

[bourdaisj]

we actually uses Float64Array a lot for gl-matrix math related stuff. so I wouldn't call it 'bad design' in this case.
if we change that in vtkImageData we would need to update everything to use plain old JS arrays to maintain consistency, and I don't think that's worth it/ Let's keep the simple fix for this PR imo.

[floryst]

The image direction matrices are a consequence of using gl-matrix for construction and allocation, since it defaults to Float32Arrays. I partially agree with @finetjul that the index/world transform matrices should be hidden from serialization, but having them is nice for downstream code to not have to always recompute the transforms upon deserialization (e.g. sending over the network).

[bourdaisj]

Thanks for review and feedback guys!
I don't know what's better. I can open an issue to keep track of this discussion. Tbh I just want to get Kitware/glance#480 fixed in this PR

[finetjul]

Let's leave it as is.

[github-actions]

🎉 This PR is included in version 28.12.4 🎉

The release is available on:

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

Your semantic-release bot 📦🚀