-
Notifications
You must be signed in to change notification settings - Fork 498
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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. | ||||||||||||
|
||||||||||||
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** | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||
* On macOS: **~/Library/Caches/pnpm/dlx** | ||||||||||||
* On Linux: **~/.cache/pnpm/dlx** | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The correct path should be The short answer is yes. |
||||||||||||
|
||||||||||||
Comment on lines
+25
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They are just a repeat of There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 this will allow for:
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, I still disagree - strongly prefer having the resolved paths
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It is just my personal habit to communicate with developers in issues and PRs. It is essentially the same as saying
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 |
||||||||||||
## Options | ||||||||||||
|
||||||||||||
### --package <name\> | ||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -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)). | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or, another idea: also link to the
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Alternativean 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
The files used by 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I see. You should create an issue and discuss it with @zkochan. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I meant regarding what you said about not easy to find a centralized location that list all these locations of |
||||||||||
|
||||||||||
The store should be always on the same disk on which installation is happening, | ||||||||||
so there will be one store per disk. If there is a home directory on the current | ||||||||||
|
There was a problem hiding this comment.
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 topnpm install
.There was a problem hiding this comment.
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