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

New command: pnpm fetch #3294

Merged
merged 24 commits into from
Apr 1, 2021
Merged

New command: pnpm fetch #3294

merged 24 commits into from
Apr 1, 2021

Conversation

thynson
Copy link
Sponsor Contributor

@thynson thynson commented Mar 30, 2021

This PR attempts to implement the idea discussed in #3207, by adding a new option --only-import-to-virtual-store (No idea for better option name yet)to install command.

With the feature implemented in this PR. Docker file can be simplified to

FROM node:lts
RUN npm install -g pnpm
COPY pnpm-lock.yaml ./ 
# This step will install all required package to the virtual store, without linking them to node_modules
RUN pnpm install --only-import-to-virtual-store
COPY . ./
RUN pnpm recursive install # Now this step only links packages

As long as the lockfile were not changed, the layer of RUN pnpm install --only-import-to-virtual-store can be cached.

@zkochan Would you please review and give some advice?

@thynson thynson requested a review from zkochan as a code owner March 30, 2021 10:08
packages/config/src/index.ts Outdated Show resolved Hide resolved
packages/headless/src/index.ts Show resolved Hide resolved
packages/config/src/Config.ts Outdated Show resolved Hide resolved
@thynson
Copy link
Sponsor Contributor Author

thynson commented Mar 30, 2021

Maybe I should rename the option from --only-import-to-virtual-store to --without-package-json or --ignore-package-json, to make its use case more clear.

@zkochan
Copy link
Member

zkochan commented Mar 30, 2021

I think --ignore-package-manifest would be good.

packages/plugin-commands-installation/src/install.ts Outdated Show resolved Hide resolved
packages/plugin-commands-installation/src/installDeps.ts Outdated Show resolved Hide resolved
packages/plugin-commands-installation/src/installDeps.ts Outdated Show resolved Hide resolved
packages/supi/src/install/index.ts Outdated Show resolved Hide resolved
@zkochan zkochan added this to the v6.0 milestone Mar 31, 2021
@zkochan
Copy link
Member

zkochan commented Mar 31, 2021

Don't forget to run pnpm -w changeset and describe your changes.

@thynson
Copy link
Sponsor Contributor Author

thynson commented Mar 31, 2021

I found that if I leave projects as is, the headless function has to be changed to skip symlink, I'm afraid the change would be too big.

@zkochan
Copy link
Member

zkochan commented Mar 31, 2021

After thinking about it more, --ignore-package-manifest seems not correct because it ignores even the project manifests that are in the lockfile, to skip direct symlinks to node_modules. Maybe something like --prepare-dependencies would be better

@thynson
Copy link
Sponsor Contributor Author

thynson commented Mar 31, 2021

I even thought it might be another low-level command that is managing the virtual store and lockfile, rather than an option of install which is just convenient for implementation.

@zkochan
Copy link
Member

zkochan commented Mar 31, 2021

Like pnpm prepare-dependencies? I don't know

cc @pnpm/collaborators

@thynson
Copy link
Sponsor Contributor Author

thynson commented Mar 31, 2021

pnpm preload?

@zkochan
Copy link
Member

zkochan commented Mar 31, 2021

We use fetch in the repository and there are settings named fetch, so probably
pnpm fetch
pnpm prefetch
pnpm fetch-dependencies

@thynson
Copy link
Sponsor Contributor Author

thynson commented Mar 31, 2021

I like pnpm prefetch,which hints it's an optmization in certain situation.

@zkochan
Copy link
Member

zkochan commented Mar 31, 2021

I've created a poll to gather some feedback https://twitter.com/pnpmjs/status/1377343518579044372

@zkochan
Copy link
Member

zkochan commented Apr 1, 2021

It looks like pnpm fetch is winning the poll.

@zkochan zkochan changed the title Import to virtual store New command: pnpm fetch Apr 1, 2021
@zkochan zkochan requested a review from a team April 1, 2021 09:14
@zkochan zkochan merged commit 735d2ac into pnpm:main Apr 1, 2021
@thynson thynson deleted the import-to-virtual-store branch April 1, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants