-
Notifications
You must be signed in to change notification settings - Fork 10
fix: onlyBuiltDependencies is an empty array
#22
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
base: main
Are you sure you want to change the base?
Conversation
|
The repository here has not been updated for a long time and there are problems with ci. Do we should continue updating here? |
| } | ||
| ): undefined | ((pkgName: string) => boolean) { | ||
| if (opts.onlyBuiltDependenciesFile || opts.onlyBuiltDependencies != null) { | ||
| if (opts.onlyBuiltDependenciesFile || opts.onlyBuiltDependencies?.length) { |
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.
This is not correct. If the array is empty it means that no dependencies are allowed to be built.
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.
This will cause all normal dependencies to be added to ignoredBuilds when executing pnpm rb, resulting in the problem mentioned in pnpm/pnpm#9129.
https://github.com/pnpm/pnpm/blob/bc0d00f46c0dbaa09c27d29b71f0d969d089eb44/exec/plugin-commands-rebuild/src/implementation/index.ts#L354
https://github.com/pnpm/pnpm/blob/bc0d00f46c0dbaa09c27d29b71f0d969d089eb44/exec/plugin-commands-rebuild/src/implementation/index.ts#L171
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.
If all builds are ignored, then pnpm rebuild should build nothing.
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.
In this case, pnpm rebuild will add all dependencies that do not need to execute lifecycle scripts to ignoredBuilds and write them to the .modules.yaml file, causing pnpm approve-builds to list these dependencies.
|
I didn't review this PR to see if it fixes the issue - but just wanted to post my comment here that I was also able to reproduce the issue that this is trying to solve: pnpm/pnpm#9129 (comment). |
refs pnpm/pnpm#9129