Use oxc_resolver for ppx path resolution #7740
Conversation
tsnobip
force-pushed
the
yarn-modern-monorepo
branch
2 times, most recently
from
724c2f8 to
c55b479
Compare
tsnobip
force-pushed
the
yarn-modern-monorepo
branch
from
96ff029 to
6ec8e86
Compare
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
tsnobip
force-pushed
the
yarn-modern-monorepo
branch
from
6ec8e86 to
af7843f
Compare
|
For a layman - will |
tsnobip
force-pushed
the
yarn-modern-monorepo
branch
from
91f0307 to
70893a1
Compare
@zth that's the aim of this PR for PPX resolution, but maybe it could be used for module resolution altogether! I haven't looked at it though! |
|
Tangentially related, but there seems to be a |
yeah, that be great. I'm interested in this as well. Already thinking about the following: once oxc has a formatter crate, we might be able to use that on the JavaScript output (optionally). |
This has been up for discussion before, IIRC. I'd personally love that, but if I remember correctly there were some objections. Anyway, worth exploring for sure. That'd let us not need to care that much about formatting when outputting from the compiler. EDIT: IMO we should lean on something like oxc as much as we need for the JS side, now that we've taken on Rust as a "dependency" for the build system. oxc and the likes are one of the advantages of the Rust ecosystem. |
tsnobip
force-pushed
the
yarn-modern-monorepo
branch
from
70893a1 to
933697c
Compare
tsnobip
force-pushed
the
yarn-modern-monorepo
branch
from
933697c to
554be65
Compare
| "-ppx".to_string(), | ||
| oxc_resolver | ||
| .resolve(root_path, y) | ||
| .unwrap() |
There was a problem hiding this comment.
As a novice Rustacean, I believe this leads to a panic when resolution failed.
However unlikely this may be, doing the .expect() will allow us to know that this part failed. So, I'm more in favour of that. Put a good message in there what we could not resolve.
| @@ -26,6 +26,7 @@ serde = { version = "1.0.152", features = ["derive"] } | |||
| serde_json = { version = "1.0.93" } | |||
| sysinfo = "0.29.10" | |||
| tempfile = "3.10.1" | |||
| oxc_resolver = "11.6.0" | |||
| package_name: &String, | ||
| ) -> Vec<String> { | ||
| pub fn flatten_ppx_flags(root_path: &Path, flags: &Option<Vec<OneOrMore<String>>>) -> Vec<String> { | ||
| let oxc_resolver = Resolver::new(ResolveOptions::default()); |
There was a problem hiding this comment.
Any idea if this is a costly thing to make?
Imagine we have 8 projects in a monorepo that all need to resolve some ppx thing. Does is make sense to recreate the oxc_resolver each time or should we have one instance of this during the lifetime of the application run?
| flags: &Option<Vec<OneOrMore<String>>>, | ||
| package_name: &String, | ||
| ) -> Vec<String> { | ||
| pub fn flatten_ppx_flags(root_path: &Path, flags: &Option<Vec<OneOrMore<String>>>) -> Vec<String> { |
There was a problem hiding this comment.
Having some second doubts here.
Is the problem not more related that a ppx flag needs an absolute path?
So, in case of a symlink we need to follow the canonical path instead?
An attempt to fix rescript-lang/rewatch#104.
I tried it on my yarn modern monorepo and it did fix PPX resolution.
I couldn't test ppx-flag with a relative path, so don't hesitate to chime in if you have this setup!