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

Clarification on Transit Nodes docs #9181

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

maschwenk
Copy link
Contributor

Description

I'm confused why this example starts off talking about linting and then the example it gives is called check-types and sounds very much like typechecking.

Unsure which one is right but it feels like it'd be better to be consistent?

Testing Instructions

Use your eyes, your intellect, and your heart

@maschwenk maschwenk requested review from anthonyshew and a team as code owners September 24, 2024 18:09
@turbo-orchestrator turbo-orchestrator bot added area: docs Improvements or additions to documentation needs: triage New issues get this label. Remove it after triage owned-by: turborepo labels Sep 24, 2024
Copy link

vercel bot commented Sep 24, 2024

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

6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 6:15pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 6:15pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 6:15pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 6:15pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 6:15pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Sep 24, 2024 6:15pm

Copy link

vercel bot commented Sep 24, 2024

@maschwenk is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@@ -326,30 +326,30 @@ Some tasks should always be ran no matter what, like a deployment script after a

Some tasks can be ran in parallel despite being dependent on other packages. An example of tasks that fit this description are linters, since a linter doesn't need to wait for outputs in dependencies to run successfully.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively you could change the above example to be typechecking instead, but based on this dissonance I'm actually a little confused by your example. Using Internal Packages™

since a type checker doesn't need to wait for outputs in dependencies to run successfully

could be true. Not using Internal Packages, where the output is .d.ts files, it would seem like you definetly can't typecheck package A if package B could be changing .d.ts files.

@ijjk
Copy link
Member

ijjk commented Sep 24, 2024

Allow CI Workflow Run

  • approve CI run for commit: 1371610

Note: this should only be enabled once the PR is ready to go and can only be enabled by a maintainer

}
}
```

This runs your tasks in parallel - but doesn't account for source code changes in dependencies. This means you can:

1. Make a breaking change to the interface of your `ui` package.
2. Run `turbo check-types`, hitting cache in an application package that depends on `ui`.
2. Run `turbo lint`, hitting cache in an application package that depends on `ui`.

This is incorrect, since the application package will show a successful cache hit, despite not being updated to use the new interface. Checking for TypeScript errors in your application package manually in your editor is likely to reveal errors.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess if I change to lint here, now these references to Typescript need changing. So probably need to understand intent better to proceed.

@@ -326,30 +326,30 @@ Some tasks should always be ran no matter what, like a deployment script after a

Some tasks can be ran in parallel despite being dependent on other packages. An example of tasks that fit this description are linters, since a linter doesn't need to wait for outputs in dependencies to run successfully.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Some tasks can be ran in parallel despite being dependent on other packages. An example of tasks that fit this description are linters, since a linter doesn't need to wait for outputs in dependencies to run successfully.
Some tasks can be ran in parallel despite being dependent on other packages. An example of tasks that fit this description are linters or type checkers, since a these tasks doesn't need to wait for outputs in dependencies to run successfully.

Technically, TypeScript as a type checker is a linter. But I can see how that would be confusing. Would this change be clearer?

Copy link
Contributor Author

@maschwenk maschwenk Sep 24, 2024

Choose a reason for hiding this comment

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

Yeah that works!

But I guess it feels like a more confusing example given my comment above:

When NOT using Internal Packages, where the output of each package is .d.ts files, it would seem like you definetly can't start typechecking package A (which depends on B) until package B emits its .d.ts files

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, missed that. You're right, let's swap the example to being lint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: docs Improvements or additions to documentation needs: triage New issues get this label. Remove it after triage owned-by: turborepo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants