-
-
Notifications
You must be signed in to change notification settings - Fork 934
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
feat: support onlyBuiltDependencies #4002
Conversation
Ci failed for the test case |
Yes. I'm debugging it. |
hi! I spent 3 hours and really cannot find out why the test fails. Can you help me? I handle the new option like |
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.
IMHO this is the requiresBuild
should be.
let requiresBuild: boolean | undefined | ||
if (options.onlyBuiltDependencies === false || options.onlyBuiltDependencies.has(options.pkg.name)) { | ||
// No onlyBuiltDependencies is specified or this package is explicitly allowed. | ||
if (options.neverBuiltDependencies.has(options.pkg.name)) { | ||
// This package is explicitly listed. | ||
requiresBuild = false | ||
} else { | ||
// default resolution | ||
requiresBuild = options.dependencyLockfile != null ? Boolean(options.dependencyLockfile.requiresBuild) : undefined | ||
} | ||
} else { | ||
requiresBuild = false | ||
} |
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.
let requiresBuild: boolean | undefined | |
if (options.onlyBuiltDependencies === false || options.onlyBuiltDependencies.has(options.pkg.name)) { | |
// No onlyBuiltDependencies is specified or this package is explicitly allowed. | |
if (options.neverBuiltDependencies.has(options.pkg.name)) { | |
// This package is explicitly listed. | |
requiresBuild = false | |
} else { | |
// default resolution | |
requiresBuild = options.dependencyLockfile != null ? Boolean(options.dependencyLockfile.requiresBuild) : undefined | |
} | |
} else { | |
requiresBuild = false | |
} | |
let requiresBuild: boolean | undefined | |
if ( | |
options.neverBuiltDependencies.has(options.pkg.name) | |
|| ( | |
options.onlyBuiltDependencies !== false | |
&& !options.onlyBuiltDependencies.has(options.pkg.name) | |
) | |
) { | |
requiresBuild = false; | |
} else if (options.dependencyLockfile != null) { | |
requiresBuild = Boolean(options.dependencyLockfile.requiresBuild); | |
} else { | |
requiresBuild = undefined; | |
} |
I'll check this week. |
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.
I'll make some refactoring
Looks like you did not allow others to push to your pull request. So I had to make another one #4014 |
thanks! |
close #4001