feat(query): reverse tracing #9319

NicholasLYang · 2024-10-23T15:46:27Z

Description

Implements an MVP of reverse tracing. Basically goes through all the JavaScript/TypeScript files in the repo, checks which ones import the reverse traced file, and returns those. Does this concurrently with a JoinSet.

Also adds code to detect if a tsconfig is in scope for a file and if so, add it to the resolver. That way custom aliases work.

Testing Instructions

Added some tests including a monorepo setup with tsconfigs in the packages that define aliases

vercel · 2024-10-23T15:46:32Z

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

Name	Status	Preview	Comments	Updated (UTC)
examples-nonmonorepo	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 29, 2024 3:21pm

8 Skipped Deployments

Name	Status	Preview	Updated (UTC)
examples-basic-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-designsystem-docs	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-gatsby-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-kitchensink-blog	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-native-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-svelte-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-tailwind-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm
examples-vite-web	⬜️ Ignored (Inspect)	Visit Preview	Oct 29, 2024 3:21pm

socket-security · 2024-10-25T17:12:16Z

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/[email protected]	Transitive: filesystem	`+25`	234 kB	jonschlinkert

View full report↗︎

NicholasLYang · 2024-10-25T18:25:45Z

crates/turbo-trace/src/tracer.rs


-            let mut parser = Parser::new_from(Capturing::new(lexer));
+    pub async fn reverse_trace(mut self) -> TraceResult {
+        let files = match globwalk::globwalk(


I'd love at some point to combine the glob walking and the import resolution so that we can have more concurrency. Right now we have to walk the entire repo, then do the import resolution step.

…ixes in a repo where we may have multiple tsconfigs

…s easy when you can make them really annoying

chris-olszewski

LGTM! Before we do any perf tinkering we should collect some profiles or set up a benchmark.

chris-olszewski · 2024-10-28T21:19:57Z

crates/turbo-trace/src/main.rs

-        for error in &result.errors {
-            eprintln!("error: {}", error);
+        for error in result.errors {
+            println!("{:?}", Report::new(error))


Is there a reason we're printing errors to stdout?

Suggested change

println!("{:?}", Report::new(error))

eprintln!("{:?}", Report::new(error))

chris-olszewski · 2024-10-28T22:43:53Z

crates/turbo-trace/src/main.rs

@@ -12,12 +13,17 @@ struct Args {
    cwd: Option<Utf8PathBuf>,
    #[clap(long)]
    ts_config: Option<Utf8PathBuf>,
+    #[clap(long)]
+    node_modules: Option<Utf8PathBuf>,


Can this be any node_modules or should it be the most specific one available?
e.g. repo/node_modules or repo/packages/foo/node_modules?

Y'know I'm not exactly sure. oxc_resolver is a little ambiguous here. This CLI isn't meant for public use, mostly just for testing

chris-olszewski · 2024-10-28T22:50:05Z

crates/turborepo-lib/src/cli/mod.rs

+    #[cfg(windows)]
+    let repo_root = repo_root.to_realpath()?;


I'd love a comment explaining why this is necessary only on Windows.

Also, we should avoid conditional compilation except where it's necessary.

Suggested change

#[cfg(windows)]

let repo_root = repo_root.to_realpath()?;

let repo_root = if cfg!(windows) {

repo_root.to_realpath()?

} else {

repo_root

};

chris-olszewski · 2024-10-29T13:26:21Z

crates/turbo-trace/src/tracer.rs

+        file_path: &AbsoluteSystemPath,
+    ) -> Option<(Vec<AbsoluteSystemPathBuf>, SeenFile)> {
+        // Read the file content
+        let Ok(file_content) = tokio::fs::read_to_string(&file_path).await else {


Not worth blocking, but I wonder if throwing rayon or a thread pool would be more effective for the reverse trace. The only concurrency we're getting is the file reads and I'm not sure how much of the weight of this function is FS vs parsing and extracting info.

yeah I'm not exactly sure. I think we should do some investigation after it gets some use

chris-olszewski · 2024-10-29T13:29:24Z

crates/turbo-trace/src/tracer.rs

+
+        // Parse the file as a module
+        let Ok(module) = parser.parse_module() else {
+            errors.push(TraceError::FileNotFound(file_path.to_owned()));


I think this should be a TraceError::ParseError?

chris-olszewski · 2024-10-29T13:39:33Z

crates/turbo-trace/src/tracer.rs

        }
    }

-    pub fn trace(mut self, max_depth: Option<usize>) -> TraceResult {
+    pub async fn get_imports_from_file(


For any future perf work it might be helpful to add some spans in this function so we can tell where time is being spent.

NicholasLYang requested a review from a team as a code owner October 23, 2024 15:46

NicholasLYang requested review from chris-olszewski and dimitropoulos October 23, 2024 15:46

turbo-orchestrator bot added created-by: turborepo labels Oct 23, 2024

NicholasLYang marked this pull request as draft October 23, 2024 15:46

NicholasLYang force-pushed the nicholasyang/reverse-turbo-trace branch from 255d821 to f86b92f Compare October 25, 2024 15:29

vercel bot deployed to Preview – examples-nonmonorepo October 25, 2024 15:29 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 25, 2024 17:10 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 25, 2024 18:24 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 25, 2024 18:30 View deployment

NicholasLYang marked this pull request as ready for review October 25, 2024 18:30

NicholasLYang commented Oct 25, 2024

View reviewed changes

vercel bot deployed to Preview – examples-nonmonorepo October 25, 2024 20:03 View deployment

NicholasLYang force-pushed the nicholasyang/reverse-turbo-trace branch from bea056c to 22f49ea Compare October 28, 2024 18:48

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 18:49 View deployment

NicholasLYang force-pushed the nicholasyang/reverse-turbo-trace branch from 22f49ea to f6e69f7 Compare October 28, 2024 18:49

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 18:50 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 18:54 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 19:21 View deployment

NicholasLYang force-pushed the nicholasyang/reverse-turbo-trace branch from c7ef347 to 1a1ccb2 Compare October 28, 2024 19:25

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 19:25 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 19:31 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 19:40 View deployment

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 19:48 View deployment

NicholasLYang added 3 commits October 28, 2024 15:50

First try at reverse tracing

2faa3fc

Add config for node modules

540aac2

Add to query

b7f1f51

NicholasLYang added 7 commits October 28, 2024 15:50

Attempting to debug some trace issues

d64902e

Reverse turbo trace with some concurrency

376f52d

Add more tests and fixing issues

4455b53

Added tsconfig inference on each file. This allows us to resolve pref…

b1e7713

…ixes in a repo where we may have multiple tsconfigs

Clippy and test fixes

02915c9

Add logging

4594096

Add an extra canonicalization step for Windows because why make thing…

971471a

…s easy when you can make them really annoying

NicholasLYang force-pushed the nicholasyang/reverse-turbo-trace branch from a2797a9 to 971471a Compare October 28, 2024 19:50

vercel bot deployed to Preview – examples-nonmonorepo October 28, 2024 19:52 View deployment

chris-olszewski approved these changes Oct 29, 2024

View reviewed changes

PR feedback

e78cdee

vercel bot deployed to Preview – examples-nonmonorepo October 29, 2024 15:21 View deployment

NicholasLYang merged commit 10f294b into main Oct 29, 2024
40 checks passed

NicholasLYang deleted the nicholasyang/reverse-turbo-trace branch October 29, 2024 16:16

renovate bot mentioned this pull request Nov 18, 2024

fix(deps): update all non-major dependencies withastro/astro#12410

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(query): reverse tracing #9319

feat(query): reverse tracing #9319

NicholasLYang commented Oct 23, 2024 •

edited

Loading

vercel bot commented Oct 23, 2024 •

edited

Loading

socket-security bot commented Oct 25, 2024 •

edited

Loading

NicholasLYang Oct 25, 2024

chris-olszewski left a comment

chris-olszewski Oct 28, 2024

chris-olszewski Oct 28, 2024

NicholasLYang Oct 29, 2024

chris-olszewski Oct 28, 2024

chris-olszewski Oct 29, 2024

NicholasLYang Oct 29, 2024

chris-olszewski Oct 29, 2024

chris-olszewski Oct 29, 2024

	println!("{:?}", Report::new(error))
	eprintln!("{:?}", Report::new(error))

feat(query): reverse tracing #9319

feat(query): reverse tracing #9319

Conversation

NicholasLYang commented Oct 23, 2024 • edited Loading

Description

Testing Instructions

vercel bot commented Oct 23, 2024 • edited Loading

socket-security bot commented Oct 25, 2024 • edited Loading

Choose a reason for hiding this comment

chris-olszewski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NicholasLYang commented Oct 23, 2024 •

edited

Loading

vercel bot commented Oct 23, 2024 •

edited

Loading

socket-security bot commented Oct 25, 2024 •

edited

Loading