-
Notifications
You must be signed in to change notification settings - Fork 471
Resolve @rescript/runtime #7858
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
Good old windows compile remove foo
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
Okay, like I mentioned on Discord, the remaining crux is that Rewatch needs to find That direct invocation doesn't set So what's next? I'm thinking of scanning for the runtime on the tooling side. If we know the workspace directory, a small traversal should find it. I prototyped this with the following script: https://gist.github.com/nojaf/f3c26f2354b1609366bb51c59bfed20e To run: node find-runtime.mjs <path-to-your-project> The script includes a fallback for Deno and worked for me on every project I tried. This is my current thinking: please try the script and report any cases where it failed to find a runtime or where performance was poor. On my Mac, all project runtimes were detected in under 5 ms. Same on Windows. |
Okay, rescript-lang/rescript-vscode#1125 is ready and I did some thorough testing on this. (ReScript 11, ReScript 12 with legacy build, ReScript 12 with Rewatch). As for the extra tests, see description of PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Making some progress here. This is an alternate take on what @cknitt had in #7637.
In short:
bsc
also gets a-runtime-path
parameter with the location of@rescript/runtime
.cli/rescript.js
determines this inresolvePackageRoot.js
. It leverages the JavaScript runtime and finds@rescript/runtime
viaimport.meta.resolve
, with fallbacks for Deno and Node require. I prefer doing these shenanigans on the JavaScript side so we can easily ask users to try tweaks in that script if needed.cli/rescript.js
setsRESCRIPT_RUNTIME
, analogous toRESCRIPT_BSC_EXE
, and Rewatch will consume it to set the-runtime-path
argument.RESCRIPT_RUNTIME
and use the regular package resolver as a fallback. It is not perfect, but should cover many cases where@rescript/runtime
is in one of the three locations checked byhelpers::try_package_path
.oxc_resolver
. Users callingnpx rescript
will have the runtime resolved by the JavaScript runtime, so in practice only ReScript maintainers will run Rewatch directly.edit
I originally set up tests that run ./cli/rescript.js with different runtimes and package managers, but that doesn't reproduce real user problems because it always runs against this repository instead of separate test repos. I want to create a lightweight testing repository instead. That will better match what end users actually have and make it much easier for people to contribute samples. The entry bar for contributing examples in this repo is higher than I'd like.