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

[WIP] Move webpack configuration to project root #382

Closed
wants to merge 1 commit into from

Conversation

ColinEberhardt
Copy link
Contributor

@ColinEberhardt ColinEberhardt commented Jan 14, 2019

As per the discussion in this issue the focus workspaces features doesn't work at the moment because webpack configuration is part of the @jpmorganchane\perspective package.

I think this configuration should be moved to the root of the project.

This PR illustrates the change for perspective-viewer only (hence it is a Work In Progress). I've simply taken the various webpack files and pasted them into scripts\webpack

With these changes, the following now works:

$ cd packages/perspective-viewer
$ yarn install --focus
$ yarn run build

If you're happy with this as an approach, I'll clean up and roll it out across the project.

@timkpaine timkpaine added the JS label Jan 14, 2019
@LukeSheard
Copy link
Contributor

@ColinEberhardt The webpack configs need to remain part of a package that's exported so that users can consume the API through webpack. Take a look at #378 which has a base for breaking these things out into a shared directory, we can make a private development package which is symlinked in but not published.

@ColinEberhardt
Copy link
Contributor Author

@ColinEberhardt The webpack configs need to remain part of a package that's exported so that users can consume the API through webpack.

I must admit, I'm not entirely sure what you mean! The APIs the Perspective currently exposes are the view and table via perspective, and the various 'viewer' WebComponents - are these the APIs you'd expect people to consume via Webpack?

@texodus
Copy link
Member

texodus commented Jan 18, 2019

@LukeSheard is correct - Perspective requires a plugin to be used via webpack due to a few features which are tricky to configure correctly without carnal knowledge of the build, namely:

  • Feature detecting WASM and loading a separate WebWorker at runtime precludes use of worker-loader
  • CORS restrictions on WebWorkers means we need to XHR the source when a cross-origin load is detected (again at runtime), and the Worker needs to be loaded via Blob.
  • As a speed optimization - the workers take ages to compile, and can't be extended in user code, so these are pre-compiled and copied to your webpack output path.
  • asm.js in particular cannot be modified by babel/uglify/etc source-to-source compilers, as none are aware of "almost asmjs" or "use asmjs" directives.
  • Shadow DOM requires some non-traditional usage of CSS/HTML assets that conflict with standard webpack configs, yielding confusing errors.

The implementation is based off Microsoft's Monaco webpack plugin, which has complications. All this aside though - I am unclear why the webpack configuration is the crux of your issues with --focus. It seems this is more an issue of the way the plugin resolves paths than the specific path of the config itself.

As I don't believe this approach is going to work, I'm going to close this PR. If you feel this is in error, feel free to re-open or come over to the gitter channel where we can discuss.

@texodus texodus closed this Jan 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants