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

Plugin API not working with globally installed modules #4000

Closed
czosel opened this issue Feb 19, 2018 · 10 comments
Closed

Plugin API not working with globally installed modules #4000

czosel opened this issue Feb 19, 2018 · 10 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program

Comments

@czosel
Copy link
Contributor

czosel commented Feb 19, 2018

Currently, globally installed plugins don't seem to be discovered by the CLI:

npm install -g prettier @prettier/plugin-php
prettier --plugin=@prettier/plugin-php --parser=php test.php
# Error: Cannot find module '@prettier/plugin-php'

npm init --yes
npm install prettier @prettier/plugin-php
prettier --plugin=@prettier/plugin-php --parser=php test.php
# works

It seems that read-pkg-up is just looking for the closest package.json file, which doesn't work for global installs

const readPkgUp = require("read-pkg-up");

(in case you're wondering why i'm explicitly passing the plugin: There also seems to be an issue with automatic discovery, running the command without results in SyntaxError: Unexpected token (see prettier/plugin-php#55))

@ikatyang
Copy link
Member

One solution is to set cwd to its own directory instead of process.cwd(), but I'm not sure if there's a side effect.

readPkgUp.sync({
normalize: false
}).pkg

@ikatyang ikatyang added the type:bug Issues identifying ugly output, or a defect in the program label Feb 19, 2018
@kachkaev
Copy link
Member

kachkaev commented Mar 19, 2018

@ikatyang looks like you're right – changing cwd helps indeed. I implemented a quick patch with this change to check your theory and was able to get Prettier plugins working inside Atom and VScode extensions (prettier/prettier-atom#395 and prettier/prettier-vscode#395) when the plugins are locally installed project dependencies.

Searching for packages in a globally installed Prettier is still not working because there is no package.json in /usr/local/lib/ (where my global node_modules are on macOS). It feels like if Prettier was searching for prettier-plugin-*/@prettier/plugin-* folders in parent node_modules rather than checking package.json, we could get a more universal solution, which would work for both local and global setups. What is also good about this approach is that plugins will be pickable when they are a part of some boilerplate packages like create-react-app. Their philosophy is to give a project creator all they want with just a single dependency in package.json.

When basedir in my hacky patch is set to process.cwd(), it equals to /path/to/current/folder for global Prettier being executed from the command line. For some reason, it is equal to / inside Atom package.

I really wish I could submit a proper PR with a fix, but after spending a couple of hours on getting to where I am, I have to admit that I won't be able to lift it. Sorry for that and hope that my experiments help you with a proper fix! Happy to try something else if that's useful.

@ntkoopman
Copy link

This also breaks on non global installs when you invoke prettier from a parent directory.

@kachkaev
Copy link
Member

kachkaev commented Mar 20, 2018

@azz WDYT of searching plugins by scanning dirs in node_modules rather than the contents of package.json? See the above comments as context.

Pinging you as the author of #3536 and #3624 😉

@azz
Copy link
Member

azz commented Mar 20, 2018

I think it's an ok solution. The use case I was originally trying to avoid was where plugins could be loaded unexpectedly (due to being intalled via transitive deps), and thus not having a way to opt out of these plugins being loaded.

I wonder how parcel bundler handles this?

@kachkaev
Copy link
Member

kachkaev commented Mar 20, 2018

IMO supporting transitive deps is a nice feature, not a thing to avoid – it'll be useful in tools similar to create-react-app. Glad that you also don't mind them (if I understood you correctly)!

I never used parcel, so unfortunately don't know how to help with your question. Who would be a good candidate for trying to submit a PR in your opinion? 🙏

@kachkaev
Copy link
Member

kachkaev commented Mar 22, 2018

Looks like I was able to fix this in #4192! 🎉 Parcel bundler seems to be OK with all this – at least yarn test:dist passes.

@kachkaev
Copy link
Member

kachkaev commented Mar 26, 2018

For those who can't wait to use prettier plugins globally while #4192 is being discussed, feel free to use a custom build derived from that PR:

npm install --global \
  github:kachkaev/prettier#5e308113217638949cc6047527d0999fe4a30de6 \
  @prettier/plugin-a \
  prettier-plugin-b

cd /any/folder
prettier '**/*.(a,b)'

This partially worked for me in prettier-atom and atom-mprettier after telling these plugins to use a global instance of Prettier. The plugins began to format code blocks in custom languages inside markdown files (more specifically, I formatted Elm code blocks via prettier-plugin-elm).

Formatting of *.a and *.b via editor extensions is not yet supported (e.g. see prettier/prettier-atom#395 and prettier/prettier-vscode#395)

@j-f1
Copy link
Member

j-f1 commented Mar 26, 2018

Shorter install name: kachkaev/prettier#fix-global-plugin-api

@kachkaev
Copy link
Member

kachkaev commented Mar 26, 2018

Or kachkaev/prettier#fix-global-plugin-api-dist (with -dist in the end) – this is the same as 5e308113217638949cc6047527d0999fe4a30de6 mentioned two comments above.

In this case the package only has pre-built files and works better in editor plugins. The sources in fix-global-plugin-api branch contain eval("require") statements, which work pretty badly in Atom (#1895).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. type:bug Issues identifying ugly output, or a defect in the program
Projects
None yet
Development

No branches or pull requests

6 participants