[internal] Add Prettier support for the experimental Javascript backend #15480
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
- Need to review how nodejs is downloaded/installed - Update NodeJS version - Update PrettierJS version # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
sureshjoshi
added
the
category:internal
sureshjoshi
changed the title
|
Good idea! I've had really bad performance when I tried running things this way two years ago (outside of Pants). I think everytime I ran, it took like 20 seconds to re-resolve. Do you know if there's a performance hit? |
That was my experience back in the day too - now it's... 100's of ms? If I've already got Prettier installed and I use the system |
Okay - probably acceptable for proof of concept, and not blocking, but generally I expect users might not be thrilled with that. Do you know if it's possible to use NPX to install the binary once, then always point to that point (probably via Digest)? |
I mean, I'll say that it feels VERY quick to me - in spite of what For Pants, I don't know if it's the resolution, or just regular pants "stuff". You can see below, that I think the overhead of calling Prettier is perfectly fine, and as a percentage of call time - NPX isn't taking up much > time ./pants_from_sources --version
2.13.0.dev1
./pants_from_sources --version 1.84s user 0.30s system 99% cpu 2.163 total> time ./pants_from_sources fmt hellojavascript/src/main.js
10:12:51.82 [WARN] Completed: pants.backend.javascript.lint.prettier.rules.prettier_fmt - prettier made changes.
hellojavascript/src/main.js
+ prettier made changes.
./pants_from_sources fmt hellojavascript/src/main.js 1.96s user 0.32s system 98% cpu 2.311 total |
- Updated with PR requested changes # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
sureshjoshi
changed the title
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| level=request.level, | ||
| env={ | ||
| "PATH": f"/bin:./{nodejs_dir}/bin", | ||
| "npm_config_cache": str(named_caches_dir.val / "npm"), # Normally stored at ~/.npm |
There was a problem hiding this comment.
@Eric-Arellano Is this all there is to do for named caches?
There was a problem hiding this comment.
Oh oops, I didn't see this. You also have to set append_only_caches. See pex_process.py for an example
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
This PR builds off of the recently added experimental Javascript targets, by adding the Prettier code formatter.
A
NodeJSsubsystem downloads the latest LTS Node runtime, which contains thenpx clitool, allowing NPM packages to be downloaded and run directly (with arguments). The way this practically works out is that we call:node {path to npx cli} prettier {prettier args}No explicit or specialized caching semantics have been considered yet, other than placing the NPM cache inside of Pant's
named_caches.(Closes #15456, #15489)
[ci skip-rust]
[ci skip-build-wheels]