Skip to content

fix: check for delete modules dir in sub-project#8967

Merged
zkochan merged 10 commits intomainfrom
install-in-sub-package-does-not-install-any-dependencies-8959
Jan 25, 2025
Merged

fix: check for delete modules dir in sub-project#8967
zkochan merged 10 commits intomainfrom
install-in-sub-package-does-not-install-any-dependencies-8959

Conversation

@KSXGitHub
Copy link
Copy Markdown
Contributor

Fixes #8959

return {
project,
manifestStats,
modulesDirStats: await modulesDirStatPromise,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This await can be delayed until modifiedProjects, but it would make the code more complicated. Should we do that?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if it is fast enough as is then I am fine if you leave it

Comment on lines +177 to +178
...project.manifest.optionalDependencies, // TODO: how to detect when to install optional?
...project.manifest.devDependencies, // TODO: handle --prod properly
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it is OK to just always check all. What is the worst that could happen? We won't have a fast repeat install? But that's only on partial install, which is a rare scenario.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is also used by verify-deps-before-run.

@KSXGitHub KSXGitHub marked this pull request as ready for review January 14, 2025 13:33
@KSXGitHub KSXGitHub requested a review from zkochan January 14, 2025 13:34
@zkochan zkochan merged commit 5c8654f into main Jan 25, 2025
@zkochan zkochan deleted the install-in-sub-package-does-not-install-any-dependencies-8959 branch January 25, 2025 18:42
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 install in a sub package does not install any dependencies

2 participants