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

Expose Node bindings for public use #1621

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

giltho
Copy link

@giltho giltho commented Oct 17, 2024

Hello,

I'm using this, your bindings are actually pretty helpful! I do need the Node bindings that are not yet expose, so I forked it, but I thought it might as well be merged.

@rgrinberg
Copy link
Contributor

Wouldn't it be better to put this in a separate package?

@giltho
Copy link
Author

giltho commented Oct 18, 2024

I created its own package at first, but I was thinking that there are already two attempts at binding node from ocaml (here and here), and if one wants to ever publish one of those on opam, this could still be published under vscode.node. The advantage and that someone writing bindings for vscode would know these bindings interact well since they are actively used here.
But I'm possibly over-thinking it, happy to move it out to its own package.

@rgrinberg
Copy link
Contributor

The main advantage of having a separate package is that users of this library will not need to build and install a bunch of unrelated stuff we use just for vscode.

You could call the package vscode-node if you'd like to avoid potential conflicts.

@giltho
Copy link
Author

giltho commented Oct 18, 2024

Hmm, the issue is that node currently depends on interop which is exposed as vscode.interop, so node would depend on vscode. Should I also create a package called vscode-interop?

@rgrinberg
Copy link
Contributor

You could put it under vscode-node.interop if you'd like. Why do the node bindings depend on vscode interop though?

@giltho
Copy link
Author

giltho commented Oct 18, 2024

It's used for:

  • Class.Make
  • Dict
  • or_undefined
  • Interface.Make

@rgrinberg
Copy link
Contributor

I see.vscode-interop.interop sounds like the right way to go

@giltho
Copy link
Author

giltho commented Oct 18, 2024

Alright done :) And it works on my machine

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

Successfully merging this pull request may close these issues.

2 participants