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: add pnpm.requiredScripts config #5802
Conversation
for (const chunk of packageChunks) { | ||
await Promise.all(chunk.map(async (prefix: string) => | ||
limitRun(async () => { | ||
const pkg = opts.selectedProjectsGraph[prefix] | ||
if (!pkg.package.manifest.scripts?.[scriptName]) { | ||
if (requiredScripts.includes(scriptName)) { | ||
throw new PnpmError('RECURSIVE_RUN_NO_SCRIPT', `Missing script "${scriptName}" in ${pkg.package.dir}`) |
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.
One observation is that this throws an error immediately in a async function, which means
- Non deterministic order
- Bad user experience to fix the errors
Let's say projects A, B are missing the target scripts:
- people may get missing script error for A, sometimes get missing script error for B.
- when a user get a missing script error for A, he/she fixes it and then run the same command again. He/She may encounter a missing script error for another project. This kind of experience feels a bit endless...
So, IMO, it will be better to print something like Missing script "${scriptName}" in packages: @my-company/A, @my-company/B
. In this way, people can get all the projects miss the target script in one operation.
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 agree
close #5569