Skip to content
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: exclude the root from recursive exec|run|add #3647

Merged
merged 3 commits into from Aug 6, 2021
Merged

feat: exclude the root from recursive exec|run|add #3647

merged 3 commits into from Aug 6, 2021

Conversation

zkochan
Copy link
Member

@zkochan zkochan commented Aug 5, 2021

close #2769

@zkochan zkochan added this to the v6.12 milestone Aug 6, 2021
@zkochan zkochan marked this pull request as ready for review August 6, 2021 00:15
@zkochan zkochan requested a review from a team August 6, 2021 00:15
Copy link
Contributor

@shellscape shellscape left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A welcome change that will let me clean up my scripts a bit 🎉

@javier-garcia-meteologica
Copy link
Contributor

It will help me clear my scripts too.

This reminds me of pnpm add <pkg> which fails to install the package in the workspace root unless -w / --workspace-root is used.

Perhaps, the flag -w / --workspace-root could be added for those that want to preserve the old behavior and include the workspace root, e.g. pnpm run -r -w <script>.

@zkochan
Copy link
Member Author

zkochan commented Aug 6, 2021

Perhaps, the flag -w / --workspace-root could be added for those that want to preserve the old behavior and include the workspace root, e.g. pnpm run -r -w <script>.

Sounds good but I also did not change the behavior of all commands. Should I?
Commands like pnpm list -r, pnpm update -r, pnpm outdated -r, etc will continue to include the workspace root.

@zkochan zkochan merged commit be2dd24 into main Aug 6, 2021
@zkochan zkochan deleted the feat/2769 branch August 6, 2021 14:28
@sebastianhelbig
Copy link

pnpm run -r -w … does not use the old behaviour and instead executes the script only on the workspace root. Expectation is that the script is run in all workspaces.

@zkochan
Copy link
Member Author

zkochan commented May 10, 2022

The -w options is changing the current working directory to the root of the workspace. That is all that that option does.

Why do you need the old behavior? I can't really come up with a useful usage example for the old behavior.

@sebastianhelbig
Copy link

Hm...okay. But using -w and -r together then doesn't have any effect at all as -w always overwrites -r.

Example project structure:

  • packages/project_1/ (with package.json)
  • packages/project_2/ (with package.json)
  • shared_directory_1
  • shared_directory_2
  • shared file_1
  • shared file_2
  • package.json

In the root package.json we have linters defined with rules global for root directories and packages. Each package itself has individual linter specifications. Normally we execute all commands in the root (via -F for scripts defined in the packages). Previously we could run -r lint to have all the linters run, the ones defined in the root and for each package. With the change this requires two commands.

@zkochan
Copy link
Member Author

zkochan commented May 10, 2022

I see, well, I liked the old behavior, but most users wanted this new behavior. And I don't know if there are many users like you, that need this.

I would suggest you to use a workaround. You can write something like this in your root package.json:

{
  "scripts": {
    "lint": "pnpm run lint-global && pnpm run -r lint",
    "lint-global": "eslint"
  }
}

So you'll be able to run one command. It will be a bit less concurrent but probably not a big deal.

@laoergege
Copy link

Why do we need the old behavior? 🤔. There may be a situation where the old project is a single repository. One day, a specific file needs to be treated as an independent package, a master-slave structure rather than a parallel structure. So, Now I want the old behavior. 😭

@zkochan
Copy link
Member Author

zkochan commented Jun 24, 2022

There will be an option to bring back the old behavior: #4928

include-workspace-root=true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Excluding the root from recursive exec (and other workspace commands)
5 participants