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

chore: remove legacy filter flags (#7970) #8240

Closed
wants to merge 28 commits into from

Conversation

mehulkar
Copy link
Contributor

Remove legacy filter flags now that they've been deprecated for quite
awhile.

Existing test suite passes. Updated tests that are still applicable and
removed those that aren't.

chris-olszewski and others added 28 commits May 28, 2024 18:25
Remove legacy filter flags now that they've been deprecated for quite
awhile.

Existing test suite passes. Updated tests that are still applicable and
removed those that aren't.
### Description

I messed up a resolving a merge conflict and didn't migrate some args
that moved from `ExecutionArgs` to `RunArgs` in #7613

### Testing Instructions

Tests on CI pass

Closes TURBO-2872
### Description

With 2.0 we will now be requiring a `packageManager` field in
`package.json` as this is a best practice and it helps us behave in a
deterministic manner.

The actual code change is very straightforward as we remove our package
manager inference code and return an error if reading package manager
from `package.json` fails.

Most of the PR is updating tests.

### Testing Instructions

Updated unit tests
### Description

Previously we would infer scope for name filters if there was exactly
one matching package.
e.g. `turbo build --filter=ui` would run `@a/ui#build` if there was a
`@a/ui` package in the workspace

This is confusing and can result in accidentally breaking filters when a
conflicting package is added e.g. adding `@b/ui` would cause *no* tasks
to be run along with a zero exit code.

### Testing Instructions

Updated unit test to verify inference no longer works and using the
explicit package name still works.
### Description

We replaced this with an option instead of an arg, but cleaning up here!

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->
### Description

We now will error if we encounter a package with a missing or empty
package name.

This is done in the graph validation step as opposed to construction
since we don't want to impose this restriction on `@turbo/repository`
users.

### Testing Instructions

Added integration test
### Description

This PR changes `--filter` so it now will error on certain malformed
filters:
- Name filters with no globs that don't match any packages e.g.
`--filter=fo` instead of `--filter=foo`
- Directory filters that reference a directory that doesn't exist: e.g.
`--filter='./pakcages/*'`

Each commit of the PR can be reviewed on it's own.

### Testing Instructions

Updated existing unit tests. Added additional ones along with an
integration test.
### Description

Moves the file system cache to `.turbo` as discussed in #842.

### Testing Instructions

Find your favorite Turborepo of choice and run a `turbo` command twice.
You should miss cache on the first run and hit cache on the second.

CLOSES TURBO-3130

---------

Co-authored-by: Chris Olszewski <chris.olszewski@vercel.com>
…on` (#8149)

### Description

Changed `outputMode` to `outputLogs` in `turbo.json` to match the flag
name. Also wrote a code mod to migrate `turbo.json` config
automatically.

### Testing Instructions

Existing tests are migrated. Also wrote tests for code mod
### Description

Change `--only` so it behaves in a more sensible way. It still shouldn't
be widely used, but now it at least does what it says it does.

Previously `--only` would still follow package dependencies resulting
additional tasks getting run that weren't expected.
e.g. for task definition `"test": {"dependsOn": ["build", "^test"]}` and
package `a` depending on `b`, then `turbo test --filter=a --only` would
result in both `a#test` and `b#test` being run. With this PR now only
`a#test` will be run.

I changed the `--only` logic so now it will limit tasks in the graph to
exactly those that are in the product of the packages implied by
`--filter` and the tasks specified in the run args. This should make
`--only` a far more sensible flag.


### Testing Instructions

Added unit tests for testing the trimming of package dependencies and
task id style dependencies e.g. `"dependsOn": ["a#test"]`
### Description

Renaming the `pipeline` key to `tasks` in turbo.json.

We went this key to better express what Turborepo does with the
configuration that is provided in this key. We've seen with users that
the term "pipeline" can be confusing for how `turbo` _really_ executes
the task graph.

In reality, the keys in the `pipeline` object are really the list of
_tasks_ registered to `turbo` - so let's call it that.

### Non-goals

This PR does not include update:
-  The examples in the repo
- JS packages
- `eslint-plugin-turbo`

These updates will be needed in subsequent PRs.

### Testing Instructions

Hopefully CI will do its job here - but, of course, would appreciate a
review of my changes to make sure I'm not missing anything my untrained
eye may be missing.

CLOSES TURBO-3225
### Description

If a user provides a glob that points to a directory e.g. `dist` or
`dist/` then we will add a trailing double star so that the directory
contents get captured instead of just the directory entry itself.

### Testing Instructions

Added unit tests for adding doublestar
Added integration test for verifying that `src/` and `dist` for task
inputs/outputs get treated as `src/**` and `dist/**` respectively.
### Description

#8180 and #8157 ended up landing in a way where `globs.t` will fail due
to using a test fixture that uses the now removed `pipeline` field.

### Testing Instructions

CI
This PR removes `globalDotEnv`/`dotEnv` in favor of `globalDependencies`
and `inputs` which is now fairly ergonomic to use with the addition of
`$TURBO_DEFAULTS$`.

This PR does *not* update the corresponding JS types.

Updated existing integration tests
### Description

Commandeered from #8151

Changes default env mode to strict and remove the "infer" option

### Testing Instructions

Existing test suite

(Currently some Windows integration tests are failing due to a missing
env var, opening for review while I work on this)

---------

Co-authored-by: nicholaslyang <nicholas.yang@vercel.com>
### Description

Handle exclusions in `globalDeps` 

### Testing Instructions

Updated `global-deps.t` to test negative globs.
### Description

TSIA

This does not change Turbopack licensing

### Testing Instructions

👀
### Description

Codemod to add missing name (or fix duplicates)
### Description

Removes the `--ignore` flag as it's interaction with other `--filter`
flags is hard to explain.

If we want to bring it back, then it should be put it into a
configuration file.

I removed the command line flag and how it feeds into the package
filtering, but left the underlying implementation of change detector
as-is. This should make it easy to bring back this behavior via a
configuration file.

### Testing Instructions

Existing tests
### Description

With this PR we will now factor in all root dependency changes. Not just
external packages.

Internal packages are handled by hashing all of the files contained in
the package directory that aren't gitignore'd.

This does have performance implications as we can end up globwalking
these directories multiple times and hashing the files multiple times if
they end up as task inputs. This will be addressed in a future PR.

### Testing Instructions

Added integration test that displays new behavior
Co-authored-by: Turbobot <turbobot@vercel.com>
### Description

Handshake logic works, we just need to bump the daemon version since
it's independent(?) of the client version.

Future work is to keep this constant in lockstep with our client version
to help us avoid forgetting to bump this.

### Testing Instructions

Verify that running binary in a repo with a running 1.x daemon will
result in the daemon getting shut down and a new one being started:
```
2024-05-28T10:17:35.750-0700 [DEBUG] turborepo_lib::daemon::connector: found pid: 6563                                                      
2024-05-28T10:17:35.750-0700 [DEBUG] turborepo_lib::daemon::connector: got daemon with pid: 6563                                                                                                                 
2024-05-28T10:17:35.766-0700 [DEBUG] turborepo_lib::daemon::connector: no pid found, starting daemon                                        
2024-05-28T10:17:35.767-0700 [DEBUG] turborepo_lib::daemon::connector: got daemon with pid: 8593
```
### Description

In #8150 we removed the deprecated
package manager argument in favor of the option, but we didn't remove
the corresponding parameter from the actual main function. This lead to
the options object getting passed as the package manager argument and
then the options object being undefined.

### Testing Instructions

Before
<img width="939" alt="Screenshot 2024-05-28 at 8 43 23 AM"
src="https://github.com/vercel/turbo/assets/4131117/6dcaa4a7-b0e0-4142-9854-95ed597d4eb5">

After
<img width="1171" alt="Screenshot 2024-05-28 at 8 43 01 AM"
src="https://github.com/vercel/turbo/assets/4131117/7ef11479-5895-4cd4-838c-a26dcaa0529a">

---------

Co-authored-by: Mehul Kar <mehul.kar@vercel.com>
Co-authored-by: Thomas Knickman <tom.knickman@vercel.com>
Co-authored-by: Anthony Shew <anthony.shew@vercel.com>
Co-authored-by: Nicholas Yang <nicholas.yang@vercel.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Turbobot <turbobot@vercel.com>
### Description

Made a nicer error message for having a `pipeline` key instead of
`tasks`.

### Testing Instructions

<!--
  Give a quick description of steps to test your changes.
-->

---------

Co-authored-by: Anthony Shew <anthony.shew@vercel.com>
This commit also updates the schema that is published to /schema.json
since it is using the same TS types that are used by code functionality
Copy link

vercel bot commented May 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 29, 2024 2:19am
examples-designsystem-docs 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 29, 2024 2:19am
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 29, 2024 2:19am
examples-svelte-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 29, 2024 2:19am
examples-vite-web 🔄 Building (Inspect) Visit Preview 💬 Add feedback May 29, 2024 2:19am
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview May 29, 2024 2:19am
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview May 29, 2024 2:19am
examples-native-web ⬜️ Ignored (Inspect) Visit Preview May 29, 2024 2:19am
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview May 29, 2024 2:19am
rust-docs ⬜️ Ignored (Inspect) Visit Preview May 29, 2024 2:19am

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

Successfully merging this pull request may close these issues.

5 participants