refactor: compile args + config to opts #9552
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Skipped Deployments
|
aef0675 to
81d04de
Compare
chris-olszewski
requested changes
Dec 3, 2024
| filter, | ||
| packages, | ||
| output, | ||
| packages, output, .. |
There was a problem hiding this comment.
This seems like a change in the wrong direction. We're now only respecting --affected if it comes from an implicit run commande.g.turbo --affected lsand ignoring the flag if it comes from thelssubcommand:turbo ls --affected`.
Plug for #9445 which prevents the former from happening
There was a problem hiding this comment.
If I did this correctly, I think I moved the logic here.
There was a problem hiding this comment.
Huh, looks like this is an existing bug? Not sure what's happening here:
[1 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ turbo --version
WARNING No locally installed `turbo` found. Using version: 2.3.4-canary.0.
2.3.4-canary.0
[0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ turbo ls --affected
WARNING No locally installed `turbo` found. Using version: 2.3.4-canary.0.
turbo 2.3.4-canary.0
WARNING ls command is experimental and may change in the future
0 no packages (pnpm)
[0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ turbo --affected ls
WARNING No locally installed `turbo` found. Using version: 2.3.4-canary.0.
turbo 2.3.4-canary.0
WARNING ls command is experimental and may change in the future
37 packages (pnpm)
@turbo-internal/top-issues-gh-action packages/top-issues
@turbo/benchmark packages/turbo-benchmark
@turbo/codemod packages/turbo-codemod
@turbo/eslint-config packages/eslint-config
@turbo/exe-stub packages/turbo-exe-stub
@turbo/gen packages/turbo-gen
@turbo/telemetry packages/turbo-telemetry
@turbo/test-utils packages/turbo-test-utils
@turbo/tsconfig packages/tsconfig
@turbo/types packages/turbo-types
@turbo/utils packages/turbo-utils
@turbo/workspaces packages/turbo-workspaces
@turborepo-examples-tests/basic turborepo-tests/example-basic
@turborepo-examples-tests/design-system turborepo-tests/example-design-system
@turborepo-examples-tests/kitchen-sink turborepo-tests/example-kitchen-sink
@turborepo-examples-tests/non-monorepo turborepo-tests/example-non-monorepo
@turborepo-examples-tests/with-berry turborepo-tests/example-with-berry
@turborepo-examples-tests/with-changesets turborepo-tests/example-with-changesets
@turborepo-examples-tests/with-npm turborepo-tests/example-with-npm
@turborepo-examples-tests/with-react-native-web turborepo-tests/example-with-react-native-web
@turborepo-examples-tests/with-rollup turborepo-tests/example-with-rollup
@turborepo-examples-tests/with-svelte turborepo-tests/example-with-svelte
@turborepo-examples-tests/with-tailwind turborepo-tests/example-with-tailwind
@turborepo-examples-tests/with-vite turborepo-tests/example-with-vite
@turborepo-examples-tests/with-yarn turborepo-tests/example-with-yarn
cargo-sweep-action .github/actions/cargo-sweep
cli cli
create-turbo packages/create-turbo
eslint-config-turbo packages/eslint-config-turbo
eslint-plugin-turbo packages/eslint-plugin-turbo
prysk packages/prysk
turbo-ignore packages/turbo-ignore
turbo-vsc packages/turbo-vsc
turborepo-examples examples
turborepo-repository packages/turbo-repository
turborepo-tests-helpers turborepo-tests/helpers
turborepo-tests-integration turborepo-tests/integration
[0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ turbo_dev ls --affected
WARNING No locally installed `turbo` found. Using version: 2.3.3.
turbo 2.3.3
WARNING ls command is experimental and may change in the future
0 no packages (pnpm)
[0 olszewski@chriss-mbp] /Users/olszewski/code/vercel/turborepo $ turbo_dev --affected ls
WARNING No locally installed `turbo` found. Using version: 2.3.3.
turbo 2.3.3
WARNING ls command is experimental and may change in the future
37 packages (pnpm)
@turbo-internal/top-issues-gh-action packages/top-issues
@turbo/benchmark packages/turbo-benchmark
@turbo/codemod packages/turbo-codemod
@turbo/eslint-config packages/eslint-config
@turbo/exe-stub packages/turbo-exe-stub
@turbo/gen packages/turbo-gen
@turbo/telemetry packages/turbo-telemetry
@turbo/test-utils packages/turbo-test-utils
@turbo/tsconfig packages/tsconfig
@turbo/types packages/turbo-types
@turbo/utils packages/turbo-utils
@turbo/workspaces packages/turbo-workspaces
@turborepo-examples-tests/basic turborepo-tests/example-basic
@turborepo-examples-tests/design-system turborepo-tests/example-design-system
@turborepo-examples-tests/kitchen-sink turborepo-tests/example-kitchen-sink
@turborepo-examples-tests/non-monorepo turborepo-tests/example-non-monorepo
@turborepo-examples-tests/with-berry turborepo-tests/example-with-berry
@turborepo-examples-tests/with-changesets turborepo-tests/example-with-changesets
@turborepo-examples-tests/with-npm turborepo-tests/example-with-npm
@turborepo-examples-tests/with-react-native-web turborepo-tests/example-with-react-native-web
@turborepo-examples-tests/with-rollup turborepo-tests/example-with-rollup
@turborepo-examples-tests/with-svelte turborepo-tests/example-with-svelte
@turborepo-examples-tests/with-tailwind turborepo-tests/example-with-tailwind
@turborepo-examples-tests/with-vite turborepo-tests/example-with-vite
@turborepo-examples-tests/with-yarn turborepo-tests/example-with-yarn
cargo-sweep-action .github/actions/cargo-sweep
cli cli
create-turbo packages/create-turbo
eslint-config-turbo packages/eslint-config-turbo
eslint-plugin-turbo packages/eslint-plugin-turbo
prysk packages/prysk
turbo-ignore packages/turbo-ignore
turbo-vsc packages/turbo-vsc
turborepo-examples examples
turborepo-repository packages/turbo-repository
turborepo-tests-helpers turborepo-tests/helpers
turborepo-tests-integration turborepo-tests/integration
chris-olszewski
approved these changes
Dec 4, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Refactors our config setup to explicitly transform the
ArgsandConfigtoOptsin theCommandBaseconstructor. This accomplishes a few things:cli. Args are structured according to the CLI parsing logic, so there are multiple fields that do the same thing (args.command.run_args and args.run_args). By compiling down toOpts, we can use a more normalized structure.Command::Runto create aRunstruct. This requires us to mock out aCommand::Runin places likequeryorls, which is unnecessary.Argsstruct viaCommandBase::args_mutbut that change is not propagated to the config because it's cached via aOnceCell.RunwithoutArgs. This is very important forquery.However, this does create a change where
CommandBase::newis now fallible. Overall this is fine, but it does mean that we sometimes error earlier than before. I couldn't figure out an alternative without makingConfigreactive on anyArgschangeTesting Instructions