Allow to require("bokehjs") in node.js #5129
Conversation
| @@ -99,11 +97,14 @@ | |||
| "proj4": "^2.3.10", | |||
| "sprintf": "^0.1.5", | |||
| "timezone": "0.0.38", | |||
| "coffee-script": "^1.9.3" | |||
There was a problem hiding this comment.
This should have been done in PR #4946.
There was a problem hiding this comment.
👍 I wondered about this when I was installing too.
|
|
||
| return load(modulePath) | ||
|
|
||
| if not (global.window? and global.document?) |
There was a problem hiding this comment.
This is to make this work in environments that provide their own web browser-like environment. Note that bokehjs requires window, document and navigator to initialize.
| return load(modulePath) | ||
|
|
||
| if not (global.window? and global.document?) | ||
| jsdom = require('jsdom').jsdom |
There was a problem hiding this comment.
Later, if/when someone tries to webpack bokeh (hint: jupyterlab, other web contexts), they're going to have this annoyingly dynamic require to deal with, when they're not reliant on it.
There was a problem hiding this comment.
@rgbkrk, to me, being able to repackage bokehjs with webpack and run bokehjs in node.js, are two different targets. There are two reasons bokehjs doesn't work out of the box in node.js. One are module aliases (to src/vendor), defined in package.json. This is something webpack should easily deal with on its own. The other is lack of appropriate environment in node.js, as bokehjs is a web browser library primarily. This shouldn't concern webpack at all. However,
if webpack requires a web browser library to be a node.js library as well, then what we can do, is to make bokehjs at least import without requiring a well set up environment. I presume this is how, e.g., jquery works in webpack.
Also, trying to repackage bokehjs in a custom way may not be as trivial as just including it in a thridparty webpack build, because some of bokehjs' functionality, like custom models, requires specific javascript bundle's encoding and additional code to be present at runtime. Note that at this point we don't use stock browserify to build bokehjs (we use a custom module labeler), and it's likely we will be using a custom approach to unify bokehjs' and custom model build, and allow for building custom, per plot, bokehjs bundles. However, if you just want to include core bokehjs' functionality, that should be easy though.
There was a problem hiding this comment.
@rgbkrk, do you have a test case on which I could experiment? In the end our build may be too complicated and there might be room for improvement.
There was a problem hiding this comment.
is to make bokehjs at least import without requiring a well set up environment.
After initial investigation, this seems to be a lot of work for little gains.
There was a problem hiding this comment.
According to webpack's documentation, it should be possible to avoid this custom require() logic by setting resolve.packageAlias = "browser".
There was a problem hiding this comment.
So, it looks like the only problem I can see right now, is that there there is one main field in package.json , but there are two conflicting entry points for either node.js and browserify webpack. If webpack can be told to override the default main, then there should be no problem at all.
|
I'm not looking to run bokeh in node proper, but in electron. In electron you have window and document - you can think if it as a browser environment where commonjs loading works. I'm asking for this ability because I'd like to be able to provide a default transform for bokeh in nteract, much like how we support geojson and Plotly: GeoJSON support: Release with screenshots (note: this is not the latest): |
Previously jsdom was used to initialize window and document if those weren't present, but this just complicates things, given the multitude of environments bokehjs is supposed to work in. Now, if window.document doesn't exist, a factory function is returned instead of immediately initializing bokehjs. This allows for proper configuration of the environment, without any assumptions on bokehjs' side.
|
@rgbkrk, OK, I got rid of |
|
Awesome! Great to see the electron app example too. |
|
There will be another release in two weeks, so this will be available as bokehjs 0.12.3. |
|
Sweet, I look forward to it! |
|
🙇 thanks again! |
It would be nice to share code with
bokehjs/test/index.coffee, but for whatever reason my attempts to do so failed. The newrequire()would work, but it isn't picked up by node.js.fixes #5125