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

JS: Move the binderhub API client to its own npm package #1689

Merged
merged 2 commits into from
May 3, 2023

Conversation

yuvipanda
Copy link
Collaborator

Many projects want / need to connect to the BinderHub API:

These all need to recreate a JS client interface to the BinderHub API.

This PR splits out the existing client API we have in the BinderHub JS codebase into its own package, and refers to that from the existing codebase. This has several advantages:

  • Multiple projects can re-use this client code easily, without having to copy-paste.
  • Separation of concerns allows us to eventually do wonderful things like add unit tests for the JS code.
  • By using the npm workspaces feature, extra complexity overhead is very minimal. Note how small this PR actually is.

As added incentive, I've added doc strings for the methods in BinderImage. I will work on refactoring the JS code in there as well, but as a separate PR.

Many projects want / need to connect to the BinderHub API:

- [thebe](https://thebe.readthedocs.io)
- [prototype UI for connecting to a private binderhub from
  profile_list](https://github.com/yuvipanda/prototype-kubespawner-dynamic-building-ui)

These all need to recreate a JS client interface to the BinderHub API.

This PR splits out the *existing* client API we have in the BinderHub
JS codebase into its own package, and refers to that from the existing
codebase. This has several advantages:

- Multiple projects can re-use this client code easily, without
  having to copy-paste.
- Separation of concerns allows us to eventually do wonderful things
  like **add unit tests** for the JS code.
- By using the npm
  [workspaces](https://docs.npmjs.com/cli/v9/using-npm/workspaces?v=true)
  feature, extra complexity overhead is very minimal. Note how small
  this PR actually is.

As added incentive, I've added doc strings for the methods in
`BinderImage`. I will work on refactoring the JS code in there as
well, but as a separate PR.
@yuvipanda
Copy link
Collaborator Author

#1373 is related, and in some ways I guess it's me picking up the work I was doing there :)

@manics manics merged commit 9e2e544 into jupyterhub:main May 3, 2023
consideRatio pushed a commit to jupyterhub/helm-chart that referenced this pull request May 3, 2023
@yuvipanda
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code:js-binderhub-client js changes to binderhub-client
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants