Skip to content
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

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

Open
dschrempf opened this issue Oct 2, 2023 · 14 comments

Comments

@dschrempf
Copy link

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Member

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:

apheleia/apheleia-core.el

Lines 755 to 771 in 791346c

(when (memq 'npx command)
(setq npx t)
(setq command (remq 'npx command)))
(when (and npx remote-match)
(when-let ((project-dir
(locate-dominating-file
default-directory "node_modules")))
(let ((binary
(expand-file-name
(car command)
(expand-file-name
".bin"
(expand-file-name
"node_modules"
project-dir)))))
(when (file-executable-p binary)
(setcar command binary)))))

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
Copy link
Author

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
Copy link
Author

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!

@dschrempf dschrempf reopened this Oct 23, 2023
@edslocomb
Copy link
Contributor

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
Copy link
Author

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

@edslocomb
Copy link
Contributor

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
Copy link
Contributor

edslocomb commented Oct 24, 2023

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
Copy link
Author

dschrempf commented Oct 25, 2023

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
Copy link
Member

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 #200 or I can do it as a follow-up.

@JordanAnthonyKing
Copy link

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: #297

@raxod502
Copy link
Member

raxod502 commented Oct 5, 2024

Okay, so now that #200 is long since merged, we can address this issue by fixing the behavior with nested node_modules directories. However I have never worked with such a project before, does anyone have an example on GitHub they can point to? Or upstream documentation on the algorithm npx uses to resolve $PATH? (This page doesn't have enough detail.)

Either way, I will research and fix the problem, eventually. But the process can be sped up if more context is available so I can test solutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants