Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bundle viewer #57

Merged
merged 3 commits into from May 3, 2022
Merged

Bundle viewer #57

merged 3 commits into from May 3, 2022

Conversation

bnmajor
Copy link
Collaborator

@bnmajor bnmajor commented Apr 25, 2022

Bundle the itk-vtk-viewer with the python package

WIP: Relies on Kitware/itk-vtk-viewer#475. Will need to update viewer version once that PR is in.

Fixes #53

@@ -8,6 +8,7 @@ import {
} from './utils'
import './style.css'
import { debounce } from 'lodash'
import 'itk-vtk-viewer'
Copy link
Member

@thewtex thewtex Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

-> something like:

import createViewer from 'itk-vtk-viewer/src/createViewer'

The idea: we do not use the exports of the package (which is pointing to a bundled version). Avoid the current difficulties bundling the bundle. In the future we will have nice ESM to work with. For now, we will also have to set up the WebPack rules similar to itk-vtk-viewer: 1) a resolve alias for itkConfig.js. 2) The vtk.js WebPack rules for building the shaders, CSS, etc.

CC @PaulHax

@bnmajor
Copy link
Collaborator Author

bnmajor commented Apr 26, 2022

Thanks for the tips @thewtex! I have been working on this some more using your suggestions but I am currently stuck on the following error:

ERROR in ./node_modules/itk-vtk-viewer/src/ViewerStore.js
Module build failed (from ./node_modules/babel-loader/lib/index.js):
SyntaxError: /home/local/KHQ/brianna.major/tensorboard-plugin-3d/client/node_modules/itk-vtk-viewer/src/ViewerStore.js: Support for the experimental syntax 'decorators-legacy' isn't currently enabled (23:3):

  21 | class MainUIStore {
  22 |   uiContainer = null
> 23 |   @observable collapsed = false
     |   ^
  24 |   @observable annotationsEnabled = true
  25 |   @observable axesEnabled = false
  26 |   @observable fullscreenEnabled = false

I've pushed a WIP commit with the most recent changes in case there is something obvious that you can see that I am doing wrong, in the meantime I'll keep looking into what's going on and reading up on webpack rules to try and understand what I'm missing.

@thewtex
Copy link
Member

thewtex commented Apr 28, 2022

@observable

@bnmajor I believe this is addressed by enabling the mobx babel preset:

https://github.com/Kitware/itk-vtk-viewer/blob/e0b5bffc8cd499b9f8ac310ba2b1fe7ee3309a9b/.babelrc#L9

@bnmajor
Copy link
Collaborator Author

bnmajor commented Apr 28, 2022

@observable

@bnmajor I believe this is addressed by enabling the mobx babel preset:

https://github.com/Kitware/itk-vtk-viewer/blob/e0b5bffc8cd499b9f8ac310ba2b1fe7ee3309a9b/.babelrc#L9

Thanks @thewtex! I did try this but still have the error, unfortunately.

@thewtex
Copy link
Member

thewtex commented Apr 29, 2022

@bnmajor is babel-preset-mobx installed?

https://github.com/Kitware/itk-vtk-viewer/blob/e0b5bffc8cd499b9f8ac310ba2b1fe7ee3309a9b/package.json#L66

Or is a different babel config being picked up?

Use babel.config rather than .babelrc so that configuration is applied project
wide rather than to a subset of the files.
@bnmajor
Copy link
Collaborator Author

bnmajor commented May 2, 2022

@bnmajor is babel-preset-mobx installed?

https://github.com/Kitware/itk-vtk-viewer/blob/e0b5bffc8cd499b9f8ac310ba2b1fe7ee3309a9b/package.json#L66

Or is a different babel config being picked up?

@thewtex Thank you for pointing this out, I had babel-preset-mobx installed but it was missing from the webpack list of presets. The last change that I was missing was to rename the babel config from .babelrc to babel.config.json in order to apply the configuration to the entire project. Thank you for all of your help, I believe this is ready for review now!

@bnmajor bnmajor marked this pull request as ready for review May 2, 2022 14:45
@bnmajor bnmajor requested a review from thewtex May 2, 2022 14:45
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ 💖

@thewtex thewtex merged commit 59f908c into KitwareMedical:main May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make work offline
2 participants