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

Bokeh npm install #5125

Closed
rgbkrk opened this issue Sep 8, 2016 · 13 comments · Fixed by #5129
Closed

Bokeh npm install #5125

rgbkrk opened this issue Sep 8, 2016 · 13 comments · Fixed by #5129

Comments

@rgbkrk
Copy link

rgbkrk commented Sep 8, 2016

I'm trying to load Bokeh in an electron context, which means I'm in node with a browser context. However, the object that require('bokehjs') returns has no methods (and isn't itself a function). Was there a packaging issue with the npm release or should I be requiring this differently?

@bryevdv
Copy link
Member

bryevdv commented Sep 8, 2016

ping @mattpap do you have any experience / ideas?

To be honest, I think the npm package gets little to possibly no use at all, so it would not surprise me if there are issues with it. But I'm not sure what would be needed to fix. Basically we build a bokeh.min.js that has Bokeh object and I think that's what's uploaded.

@rgbkrk
Copy link
Author

rgbkrk commented Sep 8, 2016

Ah, sure enough. That's sets a global Bokeh object.

screen shot 2016-09-08 at 3 55 14 pm

@rgbkrk
Copy link
Author

rgbkrk commented Sep 8, 2016

Yeah, I'm hoping for a commonjs/ES6 export that could be loaded if used with webpack or browserify.

const Bokeh = require('bokehjs');

@rgbkrk
Copy link
Author

rgbkrk commented Sep 8, 2016

I have suggestions for the build output, but I'm not sure what context this is currently used in. This built file looks like a browser dist rather than an npm package.

@bryevdv
Copy link
Member

bryevdv commented Sep 8, 2016

If you know how to do that I'm open to suggestion :) @mattpap might know how to accomplish this

@bryevdv
Copy link
Member

bryevdv commented Sep 8, 2016

That's exactly what it is. As I said AFAIK you may be the first person to try and use the npmjs package.

@bryevdv
Copy link
Member

bryevdv commented Sep 8, 2016

We upload the dist to CDN and also ship it with the python library. We don't use the npm package for anything so changing it however seems fair game.

@mattpap
Copy link
Contributor

mattpap commented Sep 9, 2016

According to #614 this is a solved issue. Anyway, main field in package.json wrongly points to build/js/bokeh.js bundle (this is where global Bokeh comes from) (looks like this was added just to shut npm up). Changing this to build/js/tree/main.js doesn't work and fails on imports from vendor. This should be hopefully easily fixable, because our tests run under node.js. Even with this fixed, require("bokehjs") won't bring widgets and plotting APIs to scope automatically. Though they can be imported separately (with some extra code as in bokehjs/test/common/defaults.coffee).

@mattpap mattpap added this to the short-term milestone Sep 9, 2016
@mattpap mattpap self-assigned this Sep 9, 2016
@mattpap
Copy link
Contributor

mattpap commented Sep 9, 2016

We can create a different entry point for npm package, that would include everything (core, widgets, APIs).

@mattpap
Copy link
Contributor

mattpap commented Sep 9, 2016

If anyone besides me wants to play with this, I couldn't find a way to actually build a npm package, just I can publish one, so I do this:

cd ~/tmp
npm install ~/repos/bokeh/bokehjs

to test the package. Installing from inside bokeh/bokehjs won't work.

@mattpap
Copy link
Contributor

mattpap commented Sep 9, 2016

This should be hopefully easily fixable, because our tests run under node.js.

Our tests use a custom module loader, so that's why vendor works there. I think we can use the same code in npm package, just at this point we have to have another entry point.

@mattpap
Copy link
Contributor

mattpap commented Sep 9, 2016

@rgbkrk, @bryevdv, PR #5129 should fix this issue.

@mattpap mattpap modified the milestones: 0.12.3, short-term Sep 9, 2016
@bryevdv
Copy link
Member

bryevdv commented Sep 9, 2016

@rgbkrk can you take a look and comment?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants