Skip to content

Commit

Permalink
feat(trace): more trace fixes (#9474)
Browse files Browse the repository at this point in the history
### Description

A couple small fixes for tracing:
- Add more extensions for support
- Add more files to look at for a main file (`module` is non-standard
but common)
- Try resolving `@types/<package>` if `<package>` fails to resolve

### Testing Instructions
Added tests for new extensions, for module field, and for importing
types from `foo` that really come from `@types/foo`
<!--
  Give a quick description of steps to test your changes.
-->
  • Loading branch information
NicholasLYang authored Nov 20, 2024
1 parent 698d5fa commit 52779f1
Show file tree
Hide file tree
Showing 14 changed files with 55 additions and 17 deletions.
7 changes: 3 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/turbo-trace/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ clap = { version = "4.5.17", features = ["derive"] }
futures = { workspace = true }
globwalk = { version = "0.1.0", path = "../turborepo-globwalk" }
miette = { workspace = true, features = ["fancy"] }
oxc_resolver = { version = "2.0.0" }
oxc_resolver = { version = "2.1.0" }
swc_common = { workspace = true, features = ["concurrent", "tty-emitter"] }
swc_ecma_ast = { workspace = true }
swc_ecma_parser = { workspace = true }
Expand Down
29 changes: 24 additions & 5 deletions crates/turbo-trace/src/tracer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,14 @@ pub enum TraceError {
span: SourceSpan,
#[source_code]
text: String,
#[help]
reason: String,
},
#[error("failed to walk files")]
GlobError(Arc<globwalk::WalkError>),
}

impl TraceResult {
#[allow(dead_code)]
pub fn emit_errors(&self) {

Check warning on line 70 in crates/turbo-trace/src/tracer.rs

View workflow job for this annotation

GitHub Actions / Rust lints

method `emit_errors` is never used

Check warning on line 70 in crates/turbo-trace/src/tracer.rs

View workflow job for this annotation

GitHub Actions / Turborepo rust check

method `emit_errors` is never used

Check warning on line 70 in crates/turbo-trace/src/tracer.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on ubuntu

method `emit_errors` is never used

Check warning on line 70 in crates/turbo-trace/src/tracer.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on macos

method `emit_errors` is never used

Check warning on line 70 in crates/turbo-trace/src/tracer.rs

View workflow job for this annotation

GitHub Actions / Turborepo Rust testing on windows

method `emit_errors` is never used
let handler = Handler::with_tty_emitter(
ColorConfig::Auto,
Expand Down Expand Up @@ -152,8 +153,7 @@ impl Tracer {
file_content.clone(),
);

let syntax = if file_path.extension() == Some("ts") || file_path.extension() == Some("tsx")
{
let syntax = if matches!(file_path.extension(), Some("ts") | Some("tsx")) {
Syntax::Typescript(TsSyntax {
tsx: file_path.extension() == Some("tsx"),
decorators: true,
Expand Down Expand Up @@ -211,6 +211,18 @@ impl Tracer {
debug!("built in: {:?}", err);
}
Err(err) => {
// Try to resolve the import as a type import via `@/types/<import>`
let type_package = format!("@types/{}", import);
let resolved_type_import = resolver
.resolve(file_path, type_package.as_str())
.ok()
.and_then(|resolved| resolved.into_path_buf().try_into().ok());

if let Some(resolved_type_import) = resolved_type_import {
files.push(resolved_type_import);
continue;
}

debug!("failed to resolve: {:?}", err);
let (start, end) = source_map.span_to_char_offset(&source_file, *span);
let start = start as usize;
Expand All @@ -221,8 +233,8 @@ impl Tracer {
file_path: file_path.to_string(),
span: SourceSpan::new(start.into(), (end - start).into()),
text: file_content.clone(),
reason: err.to_string(),
});
continue;
}
}
}
Expand Down Expand Up @@ -316,7 +328,14 @@ impl Tracer {
.with_force_extension(EnforceExtension::Disabled)
.with_extension(".ts")
.with_extension(".tsx")
.with_module(self.cwd.join_component("node_modules").to_string())
.with_extension(".jsx")
.with_extension(".d.ts")
.with_extension(".mjs")
.with_extension(".cjs")
// Some packages export a `module` field instead of `main`. This is non-standard,
// but was a proposal at some point.
.with_main_field("module")
.with_main_field("types")
// Condition names are used to determine which export to use when importing a module.
// We add a bunch so oxc_resolver can resolve all kinds of imports.
.with_condition_names(&["import", "require", "node", "default"]);
Expand Down
3 changes: 3 additions & 0 deletions crates/turborepo-lib/src/query/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ impl File {
#[derive(SimpleObject, Debug, Default)]
pub struct TraceError {
message: String,
reason: String,
path: Option<String>,
import: Option<String>,
start: Option<usize>,
Expand Down Expand Up @@ -113,6 +114,7 @@ impl From<turbo_trace::TraceError> for TraceError {
span,
text,
file_path,
reason,
..
} => {
let import = text
Expand All @@ -123,6 +125,7 @@ impl From<turbo_trace::TraceError> for TraceError {
TraceError {
message,
import,
reason,
path: Some(file_path),
start: Some(span.offset()),
end: Some(span.offset() + span.len()),
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo/tests/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn test_trace_on_monorepo() -> Result<(), anyhow::Error> {
"query",
"get `apps/my-app/index.ts` with dependencies" => "query { file(path: \"apps/my-app/index.ts\") { path dependencies { files { items { path } } errors { items { message } } } } }",
"get `packages/utils/index.ts` with dependents" => "query { file(path: \"packages/utils/index.ts\") { path dependents { files { items { path } } errors { items { message } } } } }",
"get `packages/another/index.js` with dependents" => "query { file(path: \"packages/another/index.js\") { path dependents { files { items { path } } errors { items { message } } } } }",
"get `packages/another/index.js` with dependents" => "query { file(path: \"packages/another/index.jsx\") { path dependents { files { items { path } } errors { items { message } } } } }",
);

Ok(())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,16 @@ expression: query_output
"path": "apps/my-app/types.ts"
},
{
"path": "packages/another/index.js"
"path": "node_modules/@types/d3-scale/index.d.ts"
},
{
"path": "node_modules/@types/d3-time/index.d.ts"
},
{
"path": "packages/another/index.jsx"
},
{
"path": "packages/module-package/my-module.mjs"
},
{
"path": "packages/utils/index.ts"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ expression: query_output
{
"data": {
"file": {
"path": "packages/another/index.js",
"path": "packages/another/index.jsx",
"dependents": {
"files": {
"items": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ expression: query_output
"path": "apps/my-app/index.ts"
},
{
"path": "packages/another/index.js"
"path": "packages/another/index.jsx"
}
]
},
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { useMyHook } from "utils/my-hook";
import ship from "utils";
import { blackbeard } from "../../packages/another/index.js";
import { blackbeard } from "../../packages/another/index.jsx";
import { Pirate } from "@/types.ts";
import { walkThePlank } from "module-package";
import type { ScalePoint } from "d3-scale";
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"maybefails": "exit 4"
},
"dependencies": {
"utils": "*"
"utils": "*",
"@types/d3-scale": "^4.0.2"
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { blackbeard } from "@/../../packages/another/index.js";
import { blackbeard } from "@/../../packages/another/index.jsx";

export interface Pirate {
ship: string;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const walkThePlank = "walk the plank matey";
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "module-package",
"module": "my-module.mjs"
}

0 comments on commit 52779f1

Please sign in to comment.