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

feat: add graph command #3302

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

jonathanmorley
Copy link
Contributor

No description provided.

@jonathanmorley jonathanmorley force-pushed the add-graph-command branch 3 times, most recently from ebb5182 to 498b761 Compare April 2, 2021 22:45
@jonathanmorley
Copy link
Contributor Author

Need to add some tests still

@zkochan
Copy link
Member

zkochan commented Apr 2, 2021

is the output supposed to be used with some 3rd party visualizer?

I am not sure this output is optimized for human reading

image

@jonathanmorley
Copy link
Contributor Author

It is intended to be used with https://en.wikipedia.org/wiki/DOT_(graph_description_language).
I have been testing with pnpm graph | dot -Tpng -o graph.png

@jonathanmorley
Copy link
Contributor Author

Of note is that this currently only graphs the packages in the workspace, and so does not contain nodes for packages outside the workspace.

@zkochan
Copy link
Member

zkochan commented Apr 7, 2021

Seems like a useful feature but could we, for now, publish it as a separate CLI?

Currently, I try to keep the pnpm CLI small, as it needs to be fast. For now, I would recommend publishing it as a separate package and in the future maybe we could make pnpm pluggable with a library of "official plugins".

Here are 2 examples of separate CLIs that are currently in this repository:
https://github.com/pnpm/pnpm/tree/main/packages/mount-modules
https://github.com/pnpm/pnpm/tree/main/packages/make-dedicated-lockfile

@jonathanmorley
Copy link
Contributor Author

jonathanmorley commented Apr 7, 2021

I can reimplement based on whatever is decided, but I think this makes more sense as a pnpm command rather than a standalone CLI, given the following:

  1. It relies on a workspace, not single packages like the other standalone CLIs
  2. It makes use of the global parameters from the pnpm CLI (e.g. filters)

@zkochan
Copy link
Member

zkochan commented Apr 8, 2021

What do others think? cc @pnpm/collaborators

If we merge it, I would definitely add to the description, that this is an experimental command. In long term, we'll probably need to add some lazy loading for the packages that are not related to installation. And maybe some API for plugins.

@nikoladev
Copy link
Contributor

I would also like this to be a separate package. Not only to keep the CLI small, but also to keep the dependencies at a lower number from a security perspective.

A good rule of thumb for me is that anything that is outside of the scope of what is necessary to run a package manager should be separate. The user should explicitly opt-in.

@delanni
Copy link

delanni commented Apr 8, 2021

It really does feel like a niche feature, and it also kind of bound to the dot format. I also think this is not a strictly PNPM cli feature, but maybe a utility

@burtonator
Copy link

dot is pretty flexible and has SVG output so just basic support for this would rock.

@Aankhen
Copy link
Member

Aankhen commented Nov 16, 2021

This is a very cool utility but I agree that it doesn’t make sense as part of the core CLI of pnpm. I’d love to see it as part of the potential library of official plugins @zkochan mentioned, because that would make it discoverable, and clearly indicate that it builds on pnpm’s features, without unnecessarily increasing the surface area of pnpm itself.

Now, I could see including it in pnpm as an experimental command if said library of plugins were only a concept to mull over for the distant future, so that the idea doesn’t wither and die in the mean time. As long as it were clearly marked, moving it to a plugin later would be perfectly reasonable. However, doing so would also shift the responsibility for maintaining it to the pnpm repository itself, which I’m not sure is logical.

@chengcyber
Copy link
Member

Agree with the fashion of being a separate package. IMHO, we need guide people how to implement feature as a pnpm plugin. It‘s a important step to build pnpm ecosystem.

@gluxon
Copy link
Member

gluxon commented Dec 23, 2021

I'm curious if updating pnpm list to traverse through workspace packages would help at all. I currently filed that as a feature request here: #4154

Not sure what the exact command would be, but I bet some variation of of pnpm list --fiilter=<workspace-filter> --json | jq ... would be able to output a flat list of dependency pairs if the feature above is implemented.

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

Successfully merging this pull request may close these issues.

None yet

8 participants