Vtk-js: string-replace-loader required to build, but not listed in dependencies

[zachmullen]

In a downstream project that builds against vtk-js, I'm seeing the following error:

ERROR in ./plugins/vtk_viewer/web_client/views/SurfaceView.js
Module not found: Error: Can't resolve 'string-replace-loader' in '/Users/zach/dev/girder'

It looks like string-replace-loader is supposed to be brought in transitively via kw-web-suite, however kw-web-suite is only listed in the devDependencies, not the dependencies.

Things required for build time should be listed in dependencies; devDependencies is for things that are required for development of vtk-js itself.

[jourdain]

That loader is not required when you are using vtk.js as a dependency.

The file in vtk-js/Utilities/config/webpack.loaders.js is given as a convenient but based on the webpack logic. The application bundling the package is responsible of defining its rules.
And mostly for those who use kw-web-suite.

Maybe we can provide a webpack1.loader-dep.js and webpack2.loader-dep.js for your use case along with a dependency list so users know what to add in their devDependencies.

[jourdain]

Moreover, if you use the prebuilt version of vtk.js, you don't need any of those loaders.

[zachmullen]

Moreover, if you use the prebuilt version of vtk.js, you don't need any of those loaders.

What about all the libraries that are listed inside dependencies? In the prebuilt case, wouldn't they be statically bundled, and hence also not required as dependencies?

[jourdain]

yes they would, which mean that nothing should be in dependencies. ;-)

In my mind I put dev tools in devDependencies and the libraries that I'm using in my code in dependencies.

[zachmullen]

One possibility would be to have a vtk.js package that is a source dist and includes build-time deps in dependencies, and a second npm package vtk.js-prebuilt for those who just want the prebuilt version. That would cleanly separate the two use cases with the least burden on downstreams.

[jourdain]

I think it is better to have a single name. After that the build tools needed could be better documented.

[zachmullen]

So downstreams using the source distribution would have to duplicate all the build-time dependencies? Would those be expected to remain stable?

[jourdain]

yes they should. The requirements are very low here (es6 loader/shader loader).
Nothing else. No JSX or any CSS for the core lib.

[jourdain]

kw-web-suite is a superset but it is overkilling for vtk.js.

[zachmullen]

Ok, cool, that's not too much, and if it remains stable within a major version, that's good.

[jourdain]

Most likely the project importing VTK.js is already picking its build tools and should reuse them.

[jourdain]

I'll try to document that usage (without kw-web-suite) when I get a chance.

[jourdain]

Probably something along those lines but without kw-web-suite.

[zachmullen]

Great, I'm trying it now in some girder plugins and will post here with results.

[zachmullen]

Reopening because I ran into one more issue that might actually be a bug. When building I'm still seeing:

ERROR in ./~/vtk.js/Sources/Rendering/Core/Actor/index.js
Module build failed: Error: Couldn't find preset "react" relative to directory "/Users/zach/dev/girder/node_modules/vtk.js"

Even though my loaders do not mention react preset at all. I believe this is due to react being listed in the .babelrc file within the vtk.js package. Should we remove that?

[jourdain]

yes should be remove... I'm also removing the dependency on module-css.

[jourdain]

You might be able to use https://github.com/Kitware/vtk-js/blob/master/Utilities/config/dependency.js with the next release (once travis is done)

[jourdain]

var entry = require.resolve('./src/index.js');

var path = require('path');
var webpack = require('webpack');
var vtkLoaders = require('vtk.js/Utilities/config/dependency.js').webpack.v1.loaders;
var pluginList = [];

if (process.env.NODE_ENV === 'production') {
  console.log('==> Production build');
  pluginList.push(new webpack.DefinePlugin({
    'process.env': {
      NODE_ENV: JSON.stringify('production'),
    },
  }));
}

module.exports = {
  plugins: pluginList,
  entry: entry,
  output: {
    path: './dist',
    filename: 'itkVtkImageViewer.js',
  },
  module: {
    preLoaders: [{
      test: /\.js$/,
      loader: 'eslint-loader',
      exclude: /node_modules/,
    }],
    loaders: [
      { test: entry, loader: 'expose?itkVtkImageViewer' },
    ].concat(vtkLoaders),
  },
  postcss: [
    require('autoprefixer')({ browsers: ['last 2 versions'] }),
  ],
  eslint: {
    configFile: '.eslintrc.js',
  },
};

[zachmullen]

Just verified that everything is working with 2.24.1. Thanks!