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

[mattpap]

fixes #8197
fixes #6513

[paddymul]

I built this branch and locally installed bokehJS as a package in a local package. I can verify that bokehJS now imports successfully without throwing relative path errors. Thank you

[paddymul]

After further experimentation, I think I found some bugs with this. I'm working on an example project that will demonstrate the difficulty that I am having.

[paddymul]

I created bokehjs-npm-example. bokehjs-npm-example has build instructions, I'm happy to turn that repo into documentation for bokeh. I started with the 8197_relative branch and added the following two lines to bokehjs/src/lib/index.ts .

import * as api                from "./api"
export {api}

This gets the bokehjs API into the npm module. I'm happy to create a pull request for this, but I'm not sure how a PR on a PR works. I'm also not sure if this adds the API to every bokeh artifact. I would think that you would want the API to be a part of the npm module.

When running the application (with code pulled from the bokehjs documentation), I get the following error

TypeError: FlatBush is not a constructor
/Users/paddy/code/bokeh/bokehjs/build/js/lib/core/util/spatial.js:19

This corresponds with

import FlatBush = require("flatbush")

in bokehjs/src/lib/core/util/spatial.ts

I will dig around and see if I can figure anything obvious out. Let me know if there are any fixes that I can help with.

[paddymul]

After more research, this seems to be related to the module type. BokehJS currently uses commonJS, npm seems to want es6 modules.

The whole build system for bokehJS is complicated and hard to follow. Could you give an overview of the intentions of the bokehJS setup. I'm working to become more familiar with typescript build setups and npm packaging in general. When looking at this project with an aim toward contributing, I have to work through the following steps for each change

1. What is the standard npm way of doing things?
2. What is the typescript standard way of doing things?
3. What is bokehJS actually doing here?
4. Why did bokehJS make those choices?
5. If I can make the change which enhances end user functionality that I want, does that mesh with 4 and will it be accepted by the maintainers?

In my view better understanding 4 is the biggest missing part from the bokeh team. The rest (especially 1 and 2) I should be able to figure out myself.

Thoughts?

[mattpap]

@bryevdv, I think this PR can be considered to be merged for 1.1. I did some additional testing on Windows to make sure we don't regress there. I also have code for bringing this functionality under test (issue #6389), but due to unreliability of npm cli commands, I will have to put some work into that.

[mattpap]

TypeError: FlatBush is not a constructor
/Users/paddy/code/bokeh/bokehjs/build/js/lib/core/util/spatial.js:19
This corresponds with
import FlatBush = require("flatbush")

That's typescript. This gets translated to var FlatBush = require("flatbush"), which is just a standard commonjs import. That line number in the exception is suspicious. On my system this would appear on line 10. As a starter I would upload spatial.js (e.g. as a gist), so that I could inspect it. Otherwise it may be a bunch of different reasons, that would be even hard to enumerate right now.

[paddymul]

My mistake in reporting the error. The error doesn't come from the call to require but from calling new FlatBush(points.length). I read somewhere that there is difference between how commonJS exports constructors vs es6, but I can't find the article right now.

Here is the code as built.
built_spatial.js

The following javascript code would work without an error

this.index = new FlatBush.default(points.length);

[paddymul]

Success!!
I replaced the FlatBush require line with the following two lines in the typescript system

import FlatBush_mod = require("flatbush")
const FlatBush = FlatBush_mod.default

This results in typescript errors on compile but the built npm package does work properly when installed. I would think you want to implement a different fix for this. The important part is that, we are really close to having a functioning npm module.

[mattpap]

The underlying problem is that flatbush uses an experimental module field in its package.json to define the ES module entry point, along with the standard main field. webpack and similar linkers seem to prefer module over main, despite its non-standard nature (this is actually the first time I encountered this, but to be fair I don't care that much for ES modules at this point). I will need to do proper research on this subject to decide whether this is a good idea for us to support the module field or not (currently I'm leaning towards the later, due to non-standard by default nature). You can fix this issue on your side by setting up your linker (assuming webpack) with:

{ resolve: { mainFields: ['main', 'module'] } }

For rollup etc. similar configuration option should be hopefully possible.

[mattpap]

I created issue #8801 for the ESM support discussion (which is unrelated to the substance of this PR).