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

fix(deploy): ensure that the deploy target is generated in the same directory #6859

Merged
merged 4 commits into from Jul 29, 2023

Conversation

await-ovo
Copy link
Member

fix #6858

@await-ovo await-ovo requested a review from zkochan as a code owner July 25, 2023 15:44
@await-ovo await-ovo force-pushed the feat-deploy-project-modules-dir branch from 78eaa32 to e6b74d7 Compare July 25, 2023 16:17
Copy link
Member

@zkochan zkochan left a comment

Choose a reason for hiding this comment

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

What was the issue with dedupePeerDependents?

Is the modules directory issue covered with a test?

I am not sure why all module directories should be updated, I'll have to look into it.

@await-ovo
Copy link
Member Author

What was the issue with dedupePeerDependents?

The original issue here should be related to pnpm --filter pattern install not working in dedupe-peer-dependents=true.

Is the modules directory issue covered with a test?

Yes, In this test, pnpm --filter project-1 deploy ./deploy will generate deploy/node_modules in the project root directory and sub-dir directory at the same time, this pr fixes it ~.

I am not sure why all module directories should be updated, I'll have to look into it.

Take pnpm filter project-1 deploy ./deployas an example, the modulesDir set here is only relative to project-1, which is ./deploy/node_modules. In the case of dedupe-peer-dependents=true by default, we will install dependencies for all projects, and eventually the dependencies of sub-dir/project-2 will be written to /sub-dir/deploy/node_modules during pnpm deploy.

So I updated the modulesDir of all projects here to ensure that the relative path is correct.

@zkochan
Copy link
Member

zkochan commented Jul 29, 2023

It is not a good fix. The issue is happening because deploy should run on a single project but when dedupe-peer-dependnents it true, all projects are selected:

if (opts.dedupePeerDependents) {
allProjectsGraph = opts.allProjectsGraph ?? createPkgGraph(allProjects, {
linkWorkspacePackages: Boolean(opts.linkWorkspacePackages),
}).graph
} else {

The easiest fix would be to always set dedupe-peer-dependnents to true, when running install during deploy.

@zkochan zkochan merged commit 78d43a8 into pnpm:main Jul 29, 2023
6 of 8 checks passed
@await-ovo
Copy link
Member Author

It is not a good fix. The issue is happening because deploy should run on a single project but when dedupe-peer-dependnents it true, all projects are selected:

I just noticed that even though dedupe-peer-dependents is set to false,deploy will installs the workspace root dependencies at the same time. Inside the repository here, we can see that ../../pruned/node_modules is generated at the same time after running pnpm --filter foo deploy ./pruned.

It looks like we need to set the modulesDir for each project separately, in addition to setting dedupe-peer-dependents to false.

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.

pnpm deploy generate multiple deploy target directories in different directories
2 participants