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

Document pnpm dlx path #580

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

Document pnpm dlx path #580

wants to merge 2 commits into from

Conversation

karlhorky
Copy link

@karlhorky karlhorky commented Sep 19, 2024

I was having trouble finding the location of packages such as create-playwright when running any of these commands (I was trying to edit them to verify that my PR to create-playwright would behave as I wanted):

pnpm dlx create-playwright

pnpx create-playwright

pnpm create playwright

Why

Use cases are developers who:

  • use pnpm (maybe using --global, maybe using dlx)
  • who want to modify the node_modules files that are currently being run

These developers need to find the files to edit. It's currently not easy or centralized to find the locations of these files.

This PR only deals with the "easy" part by explicitly listing out the resolved paths to these locations.

Alternatives Considered

A single page which ONLY lists out all directory paths that pnpm uses (regardless of whether it's the files or symlinks), so that it's centralized in one place.

It could potentially replace the paths listed on various sections of the npmrc docs page.

Bigger PR though.

cc @KSXGitHub I think this may have been implemented in pnpm/pnpm#7835

Copy link

stackblitz bot commented Sep 19, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

Copy link

vercel bot commented Sep 19, 2024

@karlhorky is attempting to deploy a commit to the pnpm Team on Vercel.

A member of the Team first needs to authorize it.


* On Windows: **~/AppData/Local/pnpm/dlx**
* On macOS: **~/Library/Caches/pnpm/dlx**
* On Linux: **~/.cache/pnpm/dlx**
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the correct path on Linux? I couldn't find where it was being retrieved / generated, after looking through the code in pnpm/pnpm#7835

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct path should be $CACHE_DIR/dlx for all platforms. The specific values of CACHE_DIR can be found here: https://pnpm.io/npmrc#cache-dir

The short answer is yes.


You may also specify which exact version of the package you'd like to use:

```
pnpm dlx create-react-app@next ./my-app
```

The temporary location of the downloaded files is:

* On Windows: **~/AppData/Local/pnpm-cache/dlx**
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this the correct path on Windows? I couldn't find where it was being retrieved / generated, after looking through the code in pnpm/pnpm#7835

@@ -238,7 +238,7 @@ After executing a dlx command, pnpm keeps a cache that omits the installation st
* On Linux: **~/.local/share/pnpm/store**
* Type: **path**

The location where all the packages are saved on the disk.
The location where all the packages are saved on the disk (exception: [`pnpm dlx`](./cli/dlx.md)).
Copy link
Author

@karlhorky karlhorky Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or, another idea: also link to the cache-dir heading on the same page:

Suggested change
The location where all the packages are saved on the disk (exception: [`pnpm dlx`](./cli/dlx.md)).
The location where all the packages are saved on the disk (exception: [cache data](#cache-dir) incl. [dlx](./cli/dlx.md)).

Copy link
Contributor

@KSXGitHub KSXGitHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems you have some misunderstanding. It is possible that there are more people who also misunderstand it. I think a PR to prevent those misunderstanding is necessary. Can you correct them?

Comment on lines +25 to +30
The temporary location of the downloaded files is:

* On Windows: **~/AppData/Local/pnpm-cache/dlx**
* On macOS: **~/Library/Caches/pnpm/dlx**
* On Linux: **~/.cache/pnpm/dlx**

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
The temporary location of the downloaded files is:
* On Windows: **~/AppData/Local/pnpm-cache/dlx**
* On macOS: **~/Library/Caches/pnpm/dlx**
* On Linux: **~/.cache/pnpm/dlx**

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are just a repeat of cache-dir section but with dlx suffix. So I don't think this part is necessary.

Copy link
Author

@karlhorky karlhorky Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope that this part will not be removed

to me, this is the main value of the PR: to explicitly list out the paths to the pnpm dlx files.

this will allow for:

  1. easy searching through the docs for possible paths for a particular package
  2. search engines + LLMs retrieving and surfacing this information for searchers, in case the docs search doesn't provide an experience that is discoverable enough

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The location of the cache dir is not necessary fixed. Maybe in the future, it will change. I don't think as a developer, we would like to duplicate our effort.

What I suggest is, instead of explicitly listing out the resolved values of the dlx cache paths, just mention that they are saved in $CACHE_DIR/dlx is enough.

Copy link
Author

@karlhorky karlhorky Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I still disagree - strongly prefer having the resolved paths

$CACHE_DIR is not clear, and indicates an environment variable, which is not observable / discoverable in the case of the default paths

The location of the cache dir is not necessary fixed. Maybe in the future, it will change. I don't think as a developer, we would like to duplicate our effort.

I think if the cache dir does change in future, it will be probably a grep + replace across the whole codebase (already needs to be updated in the cache-dir section on the .npmrc docs page), so I would argue, probably not extra effort to update the docs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$CACHE_DIR is not clear, and indicates an environment variable, which is not observable / discoverable in the case of the default paths

It is just my personal habit to communicate with developers in issues and PRs. It is essentially the same as saying $THIS_YEAR on Reddit. I do not suggest you use $CACHE_DIR literally. You can change it to "a subdirectory named dlx inside the [cache-dir](link)" or something.

it will be probably a grep + replace across the whole codebase

No one is expected to grep + replace everything all the time, so unless the person who would edit it remember the duplicated docs (which is unlikely), they are going to forget it.

Copy link
Author

@karlhorky karlhorky Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can change it to "a subdirectory named dlx inside the [cache-dir](link)" or something.

this is somewhat better, but still leaves out parts of the paths, which may be what users are searching for

I still strongly believe that the tradeoff is worth it - to me, the value of explicit, resolved paths far outweighs the possible future developer error because of the duplication

@@ -238,7 +238,7 @@ After executing a dlx command, pnpm keeps a cache that omits the installation st
* On Linux: **~/.local/share/pnpm/store**
* Type: **path**

The location where all the packages are saved on the disk.
The location where all the packages are saved on the disk (exception: [`pnpm dlx`](./cli/dlx.md)).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no exception. The files for dlx are still stored in the store directory. They are reflinked or hardlinked from the store directory to the dlx cache (same as when you pnpm install on one of your Node.js projects). In fact, pnpm dlx is just pnpm install on a temporary dlx cache directory.

Suggested change
The location where all the packages are saved on the disk (exception: [`pnpm dlx`](./cli/dlx.md)).
The location where all the packages are saved on the disk.

Copy link
Author

@karlhorky karlhorky Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the pnpm team thinks that there is no need to clarify in this position in the docs, then feel free to remove this change of mine

I added it because I think it's common to run into the "store location" docs when searching for locations for where pnpm stores files

but users won't always be looking for these particular paths, so linking elsewhere, where other paths can be found was my aim here

Alternative

an alternative would be a single page which ONLY listed out directory paths that pnpm used (regardless of whether it's the files or symlinks), so that it's centralized in one place...

bigger PR though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the overarching concern is when cleaning up disk spaces, the user needs to know the locations to remove?

There are 3 types of locations need to be concern about:

  • Your projects' node_modules.
  • The store directory.
  • The cache directory.

The files used by dlx are well within the 3 confines above (technically 2). And if you remove the 3 above, you should clean up dlx.

Maybe you can document the 3 locations above (if the documentation doesn't have it already). But to word it as an "exception" would confuse the users.

Copy link
Author

@karlhorky karlhorky Sep 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the overarching concern is when cleaning up disk spaces, the user needs to know the locations to remove?

no, I wasn't concerned with cleaning up disk space

my use case (mentioned in the PR description above) is to find the location of the files that are currently running when running the dlx commands, so that I can modify the files:

I was having trouble finding the location of packages such as create-playwright when running any of these commands (I was trying to edit them to verify that my PR to create-playwright would behave as I wanted):

pnpm dlx create-playwright

pnpx create-playwright

pnpm create playwright

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, dlx is still not an exception here. It still uses the same store-dir and its "projects" are still in the same cache-dir.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean to suggest that dlx was an exception

I'm trying to describe use cases of developers who:

  1. use pnpm (maybe using --global, maybe using dlx)
  2. who want to modify the node_modules files that are currently being run

the problems I'm trying to alleviate with this PR (maybe not clear, I'll copy this above to the PR description):

these developers need to find the files. it's currently not easy and centralized to find the locations of these files.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's currently not easy and centralized to find the locations of these files.

I see. You should create an issue and discuss it with @zkochan.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I edited the PR description to provide a clearer picture into the motivations.

If it needs to be copied to an issue, I can do this too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it needs to be copied to an issue, I can do this too.

I meant regarding what you said about not easy to find a centralized location that list all these locations of --global, dlx, etc. I didn't mean just dlx.

@@ -14,14 +14,20 @@ needing to install it under another project, you can run:
pnpm dlx create-react-app ./my-app
```

This will fetch `create-react-app` from the registry and run it with the given arguments.
This will fetch `create-react-app` from the registry, download it to a temporary location (see below) and run it with the given arguments.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pnpm dlx will download it to the store then reflink the files from the store to the dlx cache dir. This works similar to pnpm install.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok right, I was unsure whether this was technically 100% correct wording here. I was trying to keep the explanation simple

@zkochan
Copy link
Member

zkochan commented Oct 7, 2024

You should not edit the temporary files created by dlx. I am not against additional docs but I also don't want the users to think it is OK to go there and make changes manually.

That is not a proper way to test your changes locally.

@karlhorky
Copy link
Author

I also don't want the users to think it is OK to go there and make changes manually.

Yeah, I understand the sentiment of not wanting to officially recommend something that users could unintentionally misuse and cause confusion. On the other hand, I think there are a lot of things that users could already misuse to cause problems for themselves.

That aside, there is also a benefit to knowing how tools work and where they are saving files. And for power users, it can be a very helpful tool to be able to quickly jump in to the "node_modules location" and modify things for testing purposes. This is a pretty common workflow.

That is not a proper way to test your changes locally.

Hmm... why do you think that? As I mentioned above, I think it's a pretty common workflow for people to edit the node_modules-type location where code from others is running, in order to verify bugfixes in their project without the overhead of cloning + building (+ any other publishing / setup steps) for the package.

@karlhorky
Copy link
Author

To be clear though, I don't intend for this PR to add language for an official pnpm recommendation that users edit those files - power users will decide to do this on their own anyway without any recommendation.

@zkochan
Copy link
Member

zkochan commented Oct 7, 2024

Hmm... why do you think that? As I mentioned above, I think it's a pretty common workflow for people to edit the node_modules-type location where code from others is running, in order to verify bugfixes in their project without the overhead of cloning + building (+ any other publishing / setup steps) for the package.

If you want to do that, then use a temporary project. Create a new directory, run pnpm init, install the package that you want to test and run the package from the temporary project. Don't modify globally installed packages. It is even worse to modify a package in the dlx cache as dlx can remove your changes at any time.

@karlhorky
Copy link
Author

If you want to do that, then use a temporary project. Create a new directory, run pnpm init, install the package that you want to test and run the package from the temporary project.

Seems like an ok alternative. But it's extra setup time and friction, and causes the newly-installed package to be run in a different way than the globally installed package. So in a lot of cases, for small fixes or prototyping, I wouldn't use this.

Don't modify globally installed packages. It is even worse to modify a package in the dlx cache as dlx can remove your changes at any time.

Interesting, I didn't know that behavior about dlx. It's something to be extra cautious about.

But about "Don't modify globally installed packages." - any other reason than "dlx can remove your changes at any time"?

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.

3 participants