-
Notifications
You must be signed in to change notification settings - Fork 8
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: onlyBuiltDependencies
is an empty array
#22
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? |
@@ -7,7 +7,7 @@ export function createAllowBuildFunction ( | |||
onlyBuiltDependenciesFile?: string | |||
} | |||
): 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