radian-software / apheleia

🌷 Run code formatter on buffer contents without moving point, using RCS patches and dynamic programming.
MIT License
523 stars 74 forks source link

Apheleia does not format buffer with `prettier` when installed locally (`npx prettier ...`) #217

Open dschrempf opened 10 months ago

dschrempf commented 10 months ago

Somehow, Apheleia does not run prettier in typescript-ts-mode, if prettier is installed locally via npm only.

In the function apheleia--run-formatter-process, apheleia--format-command is run to format the command. apheleia--format-command removes the npx prefix (npx -> nil), and so, (executable-find (car command) searches for prettier instead of npx prettier. If prettier is not in path, the buffer is not formatted.

Is this behavior desired? It means that I have to install a global version of prettier that may not be identical to the one used in the project at hand.

Thanks for looking into this!

edslocomb commented 9 months ago

It sounds like you might be using prettier in your project without adding it as a dependency to your package.json?

in apheleia--format-command, when the symbol 'npx is the first element of the command list, the function removes it and then looks for the next element of the list at project_dir/node_modules/.bin/[next element]. I then uses that full path to invoke the command, falling back to a global install.

if npx prettier is run without adding prettier as a project dependency, the executable is cached elsewhere, so apheleia won't find it.

Apheleia's internal virtual command 'npx is doing "try to find the command in node_modules first, and then fall back to a global install" not "invoke the command via npx."

dschrempf commented 9 months ago

Thanks for taking your time and for explaining.

I checked the project, and we are setting prettier in devDependencies. The link to the binary is also present in /node_modules/.bin/prettier although it is a relative link to ../prettier/bin/prettier.cjs.

Now, we have several sub-projects, and all of them specify prettier as a dependency, but could it be that the paths are not correctly resolved due to the relative link? That's just a wild guess. I could try to further debug the issue, if you prefer.

edslocomb commented 9 months ago

Symlinks in node_modules/.bin are normal I think? So probably not that...

What version of apheleia are you using? Looks like typescript-ts-mode was added in 3.2

If it's not that, give me a little more detail about your setup and I'll see what I can do?

raxod502 commented 9 months ago

Apheleia should definitely use your Prettier binary installed at node_modules/.bin/prettier if it exists, if it is not doing that, then there is a bug. As long as the file at that location reads as executable then it should be used, symlinks should be fine. You can see the relevant logic here:

https://github.com/radian-software/apheleia/blob/791346cd3a331b7a6b949c69e7680f8dc389d3a3/apheleia-core.el#L755-L771

It's pretty straightforward - check if node_modules exists in a parent directory, if so, expand node_modules/.bin/prettier and check if it's executable. Sounds like something there is going wrong. We may be able to add more logging to assist in troubleshooting issues like this.

dschrempf commented 9 months ago

Hm, I just checked again, and all is working well, even when I do not install prettier globally. I am not sure if something has changed in the meantime, or if this problem has always been on my side.. Thanks a lot for explaining the details. They will help if I encounter this problem again!

dschrempf commented 9 months ago

Sorry, I spoke too soon. The issue came back, and I found the reason:

The line locate-dominating-file searches for the first directory up the tree containing "node_modules". In this project, we have sub-projects containing node_modules but not prettier, which is only a dependency of encompassing project(s).

Is this a misconfiguration of the project? (I do not really think so). Or is this a limitation of apheleia which should be addressed? Why don't we just use npx prettier ...? (I think there were some limitations, but I can not recall them).

Thanks!

edslocomb commented 9 months ago

Are you using npm workspaces in your monorepo?

iirc the issue with using npx prettier is latency, something about npx caching packages outside node_modules, and sometimes downloading before executing? npx isn't just an alias for npm run

dschrempf commented 9 months ago

Yes, we are using npm workspaces. The plan is to move towards monorepo tooling, but we are not there yet.

edslocomb commented 9 months ago

Hmm, in my limited experience repos using npm workspaces have a single node_modules dir at the root of the repo, rather than multiple node_modules dirs, one for each workspace's package.json. Sounds like you've got things set up differently?

edslocomb commented 9 months ago

fwiw with a quick test I found npx --no prettier foo.js takes ~475ms to execute, while ./node_modules/prettier/bin/prettier.cjs foo.js ran in ~175 ms, avoiding that extra 300ms latency is (I think?) why Apheleia looks for the executable instead of deferring to npx

dschrempf commented 9 months ago

I just had a look, and when using workspaces, npm install creates node_modules in all workspace subdirectories. So I think this behavior is the default.

raxod502 commented 9 months ago

Ok, so we can definitely adjust the behavior of Apheleia to handle the case of multiple node_modules correctly. As @edslocomb says, getting the formatter running with lower latency is the reason I implemented this myself rather than just using npx. (Well, that and the fact that npx is kind of unusable on account of it automatically trying to install extra packages if the command isn't present already, with no way to disable that behavior.)

We should be able to tackle that as part of https://github.com/radian-software/apheleia/pull/200 or I can do it as a follow-up.

JordanAnthonyKing commented 6 days ago

I also seem to be running into this issue, mono-repo, prettier works from npx, not from emacs, no errors given.

Could it also be relevant to: https://github.com/radian-software/apheleia/issues/297