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

Relativize module paths in generated js/d.ts files #8197

Open
tavinashb opened this Issue Aug 31, 2018 · 12 comments

Comments

Projects
None yet
5 participants
@tavinashb
Copy link

tavinashb commented Aug 31, 2018

Hi,

I am trying to use standalone BokehJS library with typescript and AngularJS.

I have installed JS library using npm i bokehjs.

and imported in component using import * as Bokeh from "bokehjs"

But after starting application getting multiple errors related to Module not found: Error: Cannot resolve module.

Here are first few lines of log -

ERROR in ./~/bokehjs/build/js/tree/document.js
Module not found: Error: Cannot resolve module 'core/bokeh_events' in
@ ./~/bokehjs/build/js/tree/document.js 7:21-49

ERROR in ./~/bokehjs/build/js/tree/protocol/message.js
Module not found: Error: Cannot resolve module 'core/util/string' in
@ ./~/bokehjs/build/js/tree/protocol/message.js 3:15-42

ERROR in ./~/bokehjs/build/js/tree/models/layouts/layout_dom.js
Module not found: Error: Cannot resolve module 'core/dom' in 
@ ./~/bokehjs/build/js/tree/models/layouts/layout_dom.js 5:12-31 
@mattpap

This comment has been minimized.

Copy link
Contributor

mattpap commented Aug 31, 2018

bokehjs uses a combination of relative and absolute imports within the library (for better or worse), so you need an additional step in the compilation and bundling processes. Here is an example how to configure TypeScript to work with bokehjs (based on the configuration of our unit tests):

    "paths": {
      "*": ["../build/js/tree/*", "../build/js/types/*", "../src/lib/external/*"]
    },

(https://github.com/bokeh/bokeh/blob/master/bokehjs/test/tsconfig.json#L16-L19). You will need to consult documentation of other tools you use to set things up there. Usually it's as simple as adding the above paths to an appropriate configuration option.

bokehjs was originally meant to be used as a JavaScript bundle, so using it as a set of node modules is currently suboptimal. This may or in fact should improve in future.

@tavinashb

This comment has been minimized.

Copy link
Author

tavinashb commented Aug 31, 2018

@mattpap thanks for reply, i am using webpack and bower in my project instead of typescript.

In this case, how to remove dependencies error as mentioned above?

Also i've removed module error by fixing require statement paths. but in this case getting
value of Bokeh.LinAlg and all other Bokeh functions as undefined.

@mattpap

This comment has been minimized.

Copy link
Contributor

mattpap commented Sep 1, 2018

but in this case getting value of Bokeh.LinAlg and all other Bokeh functions as undefined

This is because Bokeh.LinAlg (and similar) are either part of bokehjs/api module or are specific to bokehjs' bundle. If you are using bokehjs as a node library, so as individual modules, you have import everything individually. If you use it as a bundle, then you can use Bokeh namespace with all the stuff pre-imported for you.

@mattpap

This comment has been minimized.

Copy link
Contributor

mattpap commented Sep 1, 2018

In this case, how to remove dependencies error as mentioned above?

You should follow https://webpack.js.org/concepts/module-resolution/#module-paths. You will need to set up resolve.modules to point to bokehjs/build/js/tree, to allow resolution of bokehjs' modules like models/layout/layout_dom, etc.).

@bryevdv bryevdv modified the milestone: short-term Sep 11, 2018

@Shekharrajak

This comment has been minimized.

Copy link

Shekharrajak commented Jan 30, 2019

Even I tried to use BokehJs in Angular application and got these kinds of errors. Actually, every path should have been relative path.

@paddymul

This comment has been minimized.

Copy link
Contributor

paddymul commented Mar 21, 2019

@mattpap Do you have objections to me making a PR for those changes? I want to use bokehJS as a standalone module too.

Are there any problems with adding things to the bokehjs npm artifact?

@mattpap

This comment has been minimized.

Copy link
Contributor

mattpap commented Mar 21, 2019

Do you have objections to me making a PR for those changes?

What changes exactly you are referring to?

@paddymul

This comment has been minimized.

Copy link
Contributor

paddymul commented Mar 21, 2019

Changing the paths as you described on Aug, 31st.

To put it another way, what thoughts or concerns does the core bokeh team have about node module that gets published to NPM?

Would they be open to a pull request that makes bokehJS usable as standalone library for an NPM based js project.

@mattpap

This comment has been minimized.

Copy link
Contributor

mattpap commented Mar 21, 2019

Changing the paths as you described on Aug, 31st.

This refers to external configurations, nothing that can be done within bokehjs. What can be done in bokehjs, is changing the build system, by adding a stage that would rewrite absolute internal module paths with relative paths (I have some preliminary work done towards this). I don't consider this an issue by itself, because it's not uncommon for large projects to have mapped module paths. However, this is something that definitively needs attention, because even recently we run into some conflicts between our modules and transcendental dependencies (though changing the build wouldn't actually solve this particular issue).

makes bokehJS usable as standalone library for an NPM based js project.

bokehjs is already usable under configuration. Module paths aside, the only other issue is that require("bokehjs") doesn't give the same API as Bokeh global from bokeh.js bundle. I'm not sure about the overall solution to this issue, because we make actually change the API of Bokeh and not load all models ahead of time or make the API lazy, etc., to improve bokeh.js' load times. This will require some thought. I'm speaking about master, because whatever is currently published to npmjs.com is outdated and irrelevant.

@paddymul

This comment has been minimized.

Copy link
Contributor

paddymul commented Mar 21, 2019

Is there a downside to rewriting all absolute internal module paths to relative paths statically in git? I'd be happy to do this.

@mattpap

This comment has been minimized.

Copy link
Contributor

mattpap commented Mar 21, 2019

The downside is that we will end up with module paths like:

import {sum} from "../../../../core/util/array"

which is a pain to work with and maintain the code. We used to use such paths and I'm definitively not going back to that. An alternative would be to split up bokehjs into more fine grained modules, but given the complexity of the project that's easier said than done and may be actually more work than it's worth. That's why I think the most effective solution is to transform source files automatically (using TypeScript compiler's AST transforms). Given that we already interact with the compiler heavily, that should be pretty easy for me to implement, though not the biggest priority right now. However, I will be making big changes to our build next month, e.g. to allow building custom bokeh.js bundles on demand, so this could be done at the same time.

@paddymul

This comment has been minimized.

Copy link
Contributor

paddymul commented Mar 21, 2019

@mattpap mattpap added this to the 1.1.1 milestone Mar 24, 2019

@mattpap mattpap changed the title BokehJS Error: Cannot resolve module Relativize module paths in generated js/d.ts files Mar 24, 2019

@mattpap mattpap referenced a pull request that will close this issue Mar 24, 2019

Open

Relativize module paths in the emitted js/d.ts files #8782

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.