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.

Sign up for GitHub

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

Jump to bottom

Allow to require("bokehjs") in node.js #5129

Merged
merged 14 commits into from
Sep 14, 2016
Merged

Allow to require("bokehjs") in node.js #5129

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
7ac6020
Remove coffee-script from bokehjs' dependencies
mattpap Sep 9, 2016
e8576ae
Move numbro from devDependencies to dependencies
mattpap Sep 9, 2016
c145921
Move hammerjs from devDependencies to dependencies
mattpap Sep 9, 2016
0c23b32
Allow to require("bokehjs") in node.js
mattpap Sep 9, 2016
06defe8
Extend node.js' Bokeh with plotting APIs
mattpap Sep 9, 2016
8f10ccf
Allow to optionally load widgets in node.js package
mattpap Sep 9, 2016
ff02e0a
Take the same approach to load all plugins
mattpap Sep 9, 2016
35e8f13
Bring coffee-script back under devDependencies
mattpap Sep 9, 2016
1ff935d
Merge remote-tracking branch 'origin/master' into mattpap/5125_npm_pa…
mattpap Sep 14, 2016
4c5c3ec
Return a factory function when window.document is not present
mattpap Sep 14, 2016
af8150d
Add bokehjs/examples/electron
mattpap Sep 14, 2016
d5c327a
Throw an error if window.document is not available in factory()
mattpap Sep 14, 2016
5475770
Rename factory() -> bokehjs()
mattpap Sep 14, 2016
cca8f21
Add link to bokehjs' style sheet to electron/index.html
mattpap Sep 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions bokehjs/package.json
View file
Open in desktop
Expand Up @@ -43,10 +43,8 @@
"gulp-if": "^2.0.0",
"merge": "^1.2.0",
"logtrap": "^0.0.1",
"hammerjs": "^2.0.4",
"mocha": "^2.2.5",
"mocha-jsdom": "^1.0.0",
"numbro": "https://github.com/bokeh/numbro.git#b4ccab0",
"phantomjs-prebuilt": "2.1.7",
"proxyquire": "1.7.10",
"run-sequence": "^1.0.0",
Expand Down Expand Up @@ -99,11 +97,14 @@
"proj4": "^2.3.10",
"sprintf": "^0.1.5",
"timezone": "0.0.38",
"coffee-script": "^1.9.3"
"hammerjs": "^2.0.4",
"numbro": "https://github.com/bokeh/numbro.git#b4ccab0",
"root-require": "^0.3.1",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should have been done in PR #4946.

Copy link

Choose a reason for hiding this comment

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

👍 I wondered about this when I was installing too.

"jsdom": "^5.6.1"
},
"scripts": {
"build": "gulp build",
"dev-build": "gulp dev-build"
},
"main": "build/js/bokeh.js"
"main": "build/js/tree/node_main.js"
}
43 changes: 43 additions & 0 deletions bokehjs/src/coffee/node_main.coffee
View file
Open in desktop
@@ -0,0 +1,43 @@
path = require "path"
assert = require "assert"
rootRequire = require("root-require")

root = rootRequire.packpath.parent()
pkg = rootRequire("./package.json")

module.constructor.prototype.require = (modulePath) ->
assert(modulePath, 'missing path')
assert(typeof modulePath == 'string', 'path must be a string')

load = (modulePath) =>
this.constructor._load(modulePath, this)

overridePath = pkg.browser[modulePath]

if overridePath?
modulePath = path.join(root, overridePath)

return load(modulePath)

if not (global.window? and global.document?)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

jsdom = require('jsdom').jsdom
Copy link

@rgbkrk rgbkrk Sep 9, 2016 •
edited

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

@mattpap any comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to webpack's documentation, it should be possible to avoid this custom require() logic by setting resolve.packageAlias = "browser".

Copy link
Contributor Author

@mattpap mattpap Sep 13, 2016 •
edited

Choose a reason for hiding this comment

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

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.


global.document = jsdom()
global.window = document.defaultView

for own name, object of global.window
if not global[name]?
global[name] = object

Bokeh = require './main'
_ = Bokeh._

APIs = require './api'
_.extend(Bokeh, _.omit(APIs, "models"))

Bokeh.require_widgets = () ->
Widgets = require './models/widgets/main'
_.extend(Bokeh, _.omit(Widgets, "models"))
Bokeh.Models.register_locations(Widgets.models)

module.exports = Bokeh