-
Notifications
You must be signed in to change notification settings - Fork 60
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
Conversation
Thank you! 😄 @cometkim can you have a quick look at this one? |
Can't we use 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 |
The reason why I didn't go with the dynamic import approach is that I think that copying the import path logic of |
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? |
@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. |
I'll look into it at some point today 👍 |
Let's merge this one and open a follow-up issue? |
VSCode extension finally supports ESM. I just noticed it. If it needs a huge refactoring, let me know 😉 |
It might be a great time to revisit various aspects of the LSP server and extension. Let's create a list and develop a plan. |
Yeah, we can merge this first for sure. |
Here's my attempt at an async rewrite: #1093 |
rescript-lang/rescript#7395 split binaries into optional platform-specific dependencies (e.g.
@rescript/linux-x64
).I've updated
utils.findPlatformPath
(used byutils.findBscExeBinary
andutils.findEditorAnalysisBinary
) to look for these platform-specific dependencies ifnode_modules/rescript/{platform}
doesn't exist.