[FEATURE] run-script with workspaces should short-circuit on script error

[johndiiorio]

Is there an existing issue for this?

• [X] I have searched the existing issues

Current Behavior

run-script with the --workspaces flag will run all workspace scripts, regardless if one script exits with exit code 1. Please see the "Steps to Reproduce" section.

Expected Behavior

I would expect that run-script would short-circuit and not run the other scripts if one fails. Alternatively, a new flag (e.g. --short-circuit) could be introduced to run-script in order to maintain backwards compatibility.

Steps To Reproduce

Consider the following scenario:

• A root package.json file has "workspaces": ["a", "b", "c"]
• a/package.json has the script "build": "exit 1"
• Both b/package.json and c/package.json have the the script "build": "exit 0"

Run npm run build --workspaces and note that all scripts are run.

Environment

• OS: Windows 10
• Node: 16.10.0
• npm: 7.24.0

[john-landgrave]

Big upvote on this one. This is kind of a dealbreaker when building a monorepo in CI? If we just execute a npm run --workspaces package (or whatever your build/bundle command is) a package can fail and the CI build will still succeed because the exit code comes out 0.

FWIW, this is also the request in this StackOverflow question.

[johndiiorio]

@john-landgrave I agree, this bug makes it impractical to build a monorepo in a CI with just npm. One possible workaround is to create a script (and add it to your root package.json) that reads the package.json workspaces array, then individually invokes npm run build --workspace <workspace> and checks the exit code before proceeding. That shouldn't be necessary though, the behavior should be built-in.

[darcyclarke]

@johndiiorio Thanks for taking the time to share your idea! New ideas are always appreciated and are better suited for our RFC repo instead since that's the right place in order to get more attention from the rest of the team and the community.

I've transferred the issue to that repo for future discussion

[hapkecom]

Hi, is there any update on this? I thinks it's still needed for the reasons described above ...

[DavidSouther]

How would you expect npm to order script execution across packages?

[ljharb]

Topologically; in order of the dependency graph. Order would need to be deterministic for siblings but wouldn't otherwise matter.

[johnnyshankman]

Order already can be done deterministically @DavidSouther as i'm using workspaces in a repo where one pkg relies on the other, and therefore must be built first.

"workspaces": [
    "packages/common",
    "packages/cli",
  ],

common will always build first, then cli.

Update: I see what he was asking about now. Like many nested.

[johnnyshankman]

@darcyclarke any updates on this feature request? seems like a pretty easy one to add, just an exit check before moving on to the next workspace. is there any in-progress PRs to review etc?

[ljharb]

@johnnyshankman darcy is no longer at npm, and npm hasn't done RFC calls in like a year, so there's not likely been any progress.

It would definitely not be easy to add; determining proper topological ordering is not a trivial task.

[johnnyshankman]

@ljharb thanks for the update.

i'm not so sure what you mean about the topologically ordering. meaning which workspace goes first? why does that need to change?

oh also, is there a better place to request?

[ljharb]

Yes, you might have 100 child workspaces, some of which have no workspace deps, but some of which are part of a complicated dependency chain. There may even be a circular dependency.

[johnnyshankman]

@ljharb how does it resolve the graph currently? why would we change that behavior? this would simply be an additional flag, no? users with complex graphs can simply not use the flag. they probably wouldn't want it anyways.

[ljharb]

What I mean is, this feature only works properly in conjunction with topological sorting, and that is nontrivial to add.

When topological sorting is used, this feature should just be on by default, no flag.

[johnnyshankman]

@ljharb ah yes i see what you mean now. yes 100% in a perfect world we do topo sort and keep this on 24/7. agreed! also thank you for the insight. makes sense now your worries. not as easy as i would imagine!

in the meantime, is it so bad to add a --fail-fast flag for --workspaces? those who have too complex of a repo for it to work would not use it

my monorepo is one layer deep but 5 pkgs wide, so this would be hugely beneficial, as each subpkg relies on the last. building all of them in order every time to the end is silly when the first one breaks.

[johnnyshankman]

also curious if you know a modern workaround that avoids a feature request!

[darcyclarke]

@darcyclarke any updates on this feature request? seems like a pretty easy one to add, just an exit check before moving on to the next workspace. is there any in-progress PRs to review etc?

I'm no longer working on npm unfortunately. That said, & fwiw, I do plan to ensure this is a capability out of the gate in my new project 🤷‍♂️

[johnnyshankman]

@darcyclarke you're an OSS legend mate! good to see ya. look forward to that.

[BastianTrifork]

Any updates on this? Spending days migrating away from Lerna, then finding this feature missing is a huge heartbreak 💔 In regards to topo sorting, i think support for circular dependencies is going to be a lot of work that will help relatively few projects. A naïve approach might get more people to adopt workspaces, since i suspect this is a dealbreaker feature for many.