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

jsapi - improvements for 3rd party consumers #5537

Closed
Tracked by #17 ...
bmingles opened this issue May 28, 2024 · 4 comments · Fixed by #6193
Closed
Tracked by #17 ...

jsapi - improvements for 3rd party consumers #5537

bmingles opened this issue May 28, 2024 · 4 comments · Fixed by #6193
Assignees
Labels

Comments

@bmingles
Copy link
Contributor

bmingles commented May 28, 2024

Improve jsapi for 3rd party consumers

CommonJS / non-browser module support
The current jsapi is exposed as 2 ES modules that have dependencies on browser apis. Certain nodejs environments like Electron and Vscode extensions only support CommonJS. They also require polyfilling a few select browser apis to make jsapi function. It would be helpful to

  1. Provide jsapi with zero browser dependencies
  2. Provide this as multiple module types, namely es + commonjs

Dynamic download of jsapi
We currently provide jsapis via urls on running DH instances. This is great since it ensures the jsapi is compatible with the server. It does, however, require some boilerplate scripting that is a bit cumbersome for a consumer.

Npm package
It seems we could address all of the above in a single npm package.
There is an initial version of this in the vscode-extension
https://github.com/deephaven/vscode-deephaven/tree/main/packages/require-jsapi

  • Handle downloading the appropriate api based on a server url
  • Provide the appropriate module format for the consumer. We should explore using esbuild as an npm module. It may be performant enough to just transpile esm -> commonjs at runtime as part of the download. Note that DHE is already commonjs (although that could change), so the package needs to be able to handle the various scenarios.
@bmingles bmingles added feature request New feature or request jsapi triage labels May 28, 2024
@bmingles bmingles self-assigned this May 28, 2024
@bmingles
Copy link
Contributor Author

@niloc132 @mofojed I created this task to facilitate some discussion / thoughts on jsapi improvements based on recent experience developing a dh vscode extension.

@bmingles
Copy link
Contributor Author

bmingles commented Jun 3, 2024

@niloc132 I investigated node support for h2c a bit. Here's my understanding:

  • node fetch uses the undici package under the hood. Http2 is not the default, but it can be enabled. It does not support h2c. Also may still be considered experimental.
  • fetch-h2 package supports h2c. There are some options for manual disconnection once requests are finished. Not sure how complicated this gets in practice

bmingles added a commit to deephaven/vscode-deephaven that referenced this issue Sep 20, 2024
- Moved the jsapi fetching code into a sub npm package
- Refactored the jsapi fetching code to have zero dependencies on
`vscode`

The new `packages` directory provides a better silo for temporary code
that will eventually become proper npm packages. e.g. enterprise jsapi
types will be put in here on subsequent PR. Should also help with early
dev of deephaven/deephaven-core#5537
@niloc132
Copy link
Member

niloc132 commented Oct 10, 2024

Concrete changes for the js api to consider (follow-up for Colin):

  • Don't require window/document on global, but create them in the js api's own Scope shim
  • Stop using Event and CustomEvent, but declare our own
  • Stop using window.location to decide if we enable websockets or not, but use the provided URL to the server (and a flag eventually, see below)

Brian will also try again with fetch-h2, assigning to global.fetch, so that the grpc-web ts implementation we have can make h2 calls, instead of requiring websockets.

@bmingles
Copy link
Contributor Author

bmingles commented Oct 10, 2024

I'm tracking the fetch / fetch-h2 testing in DHE: Connect to Core+ worker over http2 #154

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

Successfully merging a pull request may close this issue.

2 participants