Skip to content

Find per-platform binaries for v12.0.0-alpha.13+ #1092

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

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

Conversation

mediremi
Copy link
Contributor

rescript-lang/rescript#7395 split binaries into optional platform-specific dependencies (e.g. @rescript/linux-x64).

I've updated utils.findPlatformPath (used by utils.findBscExeBinary and utils.findEditorAnalysisBinary) to look for these platform-specific dependencies if node_modules/rescript/{platform} doesn't exist.

@zth
Copy link
Collaborator

zth commented May 11, 2025

Thank you! 😄

@cometkim can you have a quick look at this one?

@cometkim
Copy link
Member

Can't we use rescript as an API?

If we already know the directory, we can use it as an API like:

const { binPaths } = await import('../path/to/rescript package')
binPaths.bsc_exe // should point to the binary path

@mediremi
Copy link
Contributor Author

The reason why I didn't go with the dynamic import approach is that findPlatformPath and its upstreams (e.g. utils.findBscExeBinary->server.openedFile->server.onMessage) are all synchronous functions and that making them async would likely be an extensive refactor.

I think that copying the import path logic of @rescript/${process.platform}-${process.arch} from cli/common/bins.js is a good pragmatic short-term solution, and a refactor to make the relevant functions async can be done later.

@nojaf
Copy link
Contributor

nojaf commented May 14, 2025

The entire LSP server is riddled with short-term fixes. I would recommend taking the plunge and going for the asynchronous rewrite. I've encountered this limitation before, and I genuinely believe it's the best approach. @zth, what are your thoughts?

@zth
Copy link
Collaborator

zth commented May 14, 2025

@mediremi would it be possible for you to do a quick assessment on how much work it'd be to do the async refactor? At some point we'll need to bite the bullet for sure. But whether it's best to do it now or later is not clear to me right now.

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.

4 participants