Skip to content

Commit

Permalink
fix(turbopack): don't parse .ts files as .tsx (#7121)
Browse files Browse the repository at this point in the history
### Description

We currently parse JSX syntax in all typescript files which is wrong.

Closes PACK-2302

Next.js PR: vercel/next.js#61219
  • Loading branch information
ForsakenHarmony committed Jan 29, 2024
1 parent 41d7573 commit 550126e
Show file tree
Hide file tree
Showing 7 changed files with 108 additions and 95 deletions.
21 changes: 16 additions & 5 deletions crates/turbopack-ecmascript/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,12 @@ pub enum EcmascriptModuleAssetType {
/// Module with EcmaScript code
Ecmascript,
/// Module with TypeScript code without types
Typescript,
/// Module with TypeScript code with references to imported types
TypescriptWithTypes,
Typescript {
// parse JSX syntax.
tsx: bool,
// follow references to imported types.
analyze_types: bool,
},
/// Module with TypeScript declaration code
TypescriptDeclaration,
}
Expand All @@ -148,8 +151,16 @@ impl Display for EcmascriptModuleAssetType {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
EcmascriptModuleAssetType::Ecmascript => write!(f, "ecmascript"),
EcmascriptModuleAssetType::Typescript => write!(f, "typescript"),
EcmascriptModuleAssetType::TypescriptWithTypes => write!(f, "typescript with types"),
EcmascriptModuleAssetType::Typescript { tsx, analyze_types } => {
write!(f, "typescript")?;
if *tsx {
write!(f, "with JSX")?;
}
if *analyze_types {
write!(f, "with types")?;
}
Ok(())
}
EcmascriptModuleAssetType::TypescriptDeclaration => write!(f, "typescript declaration"),
}
}
Expand Down
10 changes: 4 additions & 6 deletions crates/turbopack-ecmascript/src/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,12 @@ async fn parse_content(
auto_accessors: true,
explicit_resource_management: true,
}),
EcmascriptModuleAssetType::Typescript
| EcmascriptModuleAssetType::TypescriptWithTypes => {
EcmascriptModuleAssetType::Typescript { tsx, .. } => {
Syntax::Typescript(TsConfig {
decorators: true,
dts: false,
no_early_errors: true,
tsx: true,
tsx,
disallow_ambiguous_jsx_like: false,
})
}
Expand All @@ -293,7 +292,7 @@ async fn parse_content(
decorators: true,
dts: true,
no_early_errors: true,
tsx: true,
tsx: false,
disallow_ambiguous_jsx_like: false,
})
}
Expand Down Expand Up @@ -330,8 +329,7 @@ async fn parse_content(

let is_typescript = matches!(
ty,
EcmascriptModuleAssetType::Typescript
| EcmascriptModuleAssetType::TypescriptWithTypes
EcmascriptModuleAssetType::Typescript { .. }
| EcmascriptModuleAssetType::TypescriptDeclaration
);
parsed_program.visit_mut_with(&mut resolver(
Expand Down
6 changes: 3 additions & 3 deletions crates/turbopack-ecmascript/src/references/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,9 @@ pub(crate) async fn analyse_ecmascript_module_internal(

// Is this a typescript file that requires analzying type references?
let analyze_types = match &*ty {
EcmascriptModuleAssetType::TypescriptWithTypes
| EcmascriptModuleAssetType::TypescriptDeclaration => true,
EcmascriptModuleAssetType::Typescript | EcmascriptModuleAssetType::Ecmascript => false,
EcmascriptModuleAssetType::Typescript { analyze_types, .. } => *analyze_types,
EcmascriptModuleAssetType::TypescriptDeclaration => true,
EcmascriptModuleAssetType::Ecmascript => false,
};

let parsed = if let Some(part) = part {
Expand Down
5 changes: 4 additions & 1 deletion crates/turbopack-mdx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,10 @@ async fn into_ecmascript_module_asset(
Ok(EcmascriptModuleAsset::new(
Vc::upcast(source),
this.asset_context,
Value::new(EcmascriptModuleAssetType::Typescript),
Value::new(EcmascriptModuleAssetType::Typescript {
tsx: true,
analyze_types: false,
}),
this.transforms,
Value::new(Default::default()),
this.asset_context.compile_time_info(),
Expand Down
35 changes: 17 additions & 18 deletions crates/turbopack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,19 +117,19 @@ async fn apply_module_type(
}
| ModuleType::Typescript {
transforms,
options,
}
| ModuleType::TypescriptWithTypes {
transforms,
tsx: _,
analyze_types: _,
options,
}
| ModuleType::TypescriptDeclaration {
transforms,
options,
} => {
let context_for_module = match module_type {
ModuleType::TypescriptWithTypes { .. }
| ModuleType::TypescriptDeclaration { .. } => {
ModuleType::Typescript { analyze_types, .. } if *analyze_types => {
module_asset_context.with_types_resolving_enabled()
}
ModuleType::TypescriptDeclaration { .. } => {
module_asset_context.with_types_resolving_enabled()
}
_ => module_asset_context,
Expand All @@ -145,11 +145,13 @@ async fn apply_module_type(
ModuleType::Ecmascript { .. } => {
builder = builder.with_type(EcmascriptModuleAssetType::Ecmascript)
}
ModuleType::Typescript { .. } => {
builder = builder.with_type(EcmascriptModuleAssetType::Typescript)
}
ModuleType::TypescriptWithTypes { .. } => {
builder = builder.with_type(EcmascriptModuleAssetType::TypescriptWithTypes)
ModuleType::Typescript {
tsx, analyze_types, ..
} => {
builder = builder.with_type(EcmascriptModuleAssetType::Typescript {
tsx: *tsx,
analyze_types: *analyze_types,
})
}
ModuleType::TypescriptDeclaration { .. } => {
builder = builder.with_type(EcmascriptModuleAssetType::TypescriptDeclaration)
Expand Down Expand Up @@ -476,16 +478,13 @@ async fn process_default_internal(
}),
Some(ModuleType::Typescript {
transforms,
tsx,
analyze_types,
options,
}) => Some(ModuleType::Typescript {
transforms: transforms.extend(*additional_transforms),
options,
}),
Some(ModuleType::TypescriptWithTypes {
transforms,
options,
}) => Some(ModuleType::TypescriptWithTypes {
transforms: transforms.extend(*additional_transforms),
tsx,
analyze_types,
options,
}),
Some(ModuleType::Mdx {
Expand Down
117 changes: 60 additions & 57 deletions crates/turbopack/src/module_options/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,67 +257,70 @@ impl ModuleOptions {
})],
),
ModuleRule::new_all(
ModuleRuleCondition::any(vec![
ModuleRuleCondition::ResourcePathEndsWith(".ts".to_string()),
ModuleRuleCondition::ResourcePathEndsWith(".tsx".to_string()),
]),
vec![if enable_types {
ModuleRuleEffect::ModuleType(ModuleType::TypescriptWithTypes {
transforms: ts_app_transforms,
options: ecmascript_options,
})
} else {
ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
options: ecmascript_options,
})
}],
ModuleRuleCondition::ResourcePathEndsWith(".ts".to_string()),
vec![ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
tsx: false,
analyze_types: enable_types,
options: ecmascript_options,
})],
),
ModuleRule::new_all(
ModuleRuleCondition::any(vec![
ModuleRuleCondition::ResourcePathEndsWith(".mts".to_string()),
ModuleRuleCondition::ResourcePathEndsWith(".mtsx".to_string()),
]),
vec![if enable_types {
ModuleRuleEffect::ModuleType(ModuleType::TypescriptWithTypes {
transforms: ts_app_transforms,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::EcmaScript,
..ecmascript_options
},
})
} else {
ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::EcmaScript,
..ecmascript_options
},
})
}],
ModuleRuleCondition::ResourcePathEndsWith(".tsx".to_string()),
vec![ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
tsx: true,
analyze_types: enable_types,
options: ecmascript_options,
})],
),
ModuleRule::new_all(
ModuleRuleCondition::any(vec![
ModuleRuleCondition::ResourcePathEndsWith(".cts".to_string()),
ModuleRuleCondition::ResourcePathEndsWith(".ctsx".to_string()),
]),
vec![if enable_types {
ModuleRuleEffect::ModuleType(ModuleType::TypescriptWithTypes {
transforms: ts_app_transforms,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::CommonJs,
..ecmascript_options
},
})
} else {
ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::CommonJs,
..ecmascript_options
},
})
}],
ModuleRuleCondition::ResourcePathEndsWith(".mts".to_string()),
vec![ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
tsx: false,
analyze_types: enable_types,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::EcmaScript,
..ecmascript_options
},
})],
),
ModuleRule::new_all(
ModuleRuleCondition::ResourcePathEndsWith(".mtsx".to_string()),
vec![ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
tsx: true,
analyze_types: enable_types,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::EcmaScript,
..ecmascript_options
},
})],
),
ModuleRule::new_all(
ModuleRuleCondition::ResourcePathEndsWith(".cts".to_string()),
vec![ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
tsx: false,
analyze_types: enable_types,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::CommonJs,
..ecmascript_options
},
})],
),
ModuleRule::new_all(
ModuleRuleCondition::ResourcePathEndsWith(".ctsx".to_string()),
vec![ModuleRuleEffect::ModuleType(ModuleType::Typescript {
transforms: ts_app_transforms,
tsx: true,
analyze_types: enable_types,
options: EcmascriptOptions {
specified_module_type: SpecifiedModuleType::CommonJs,
..ecmascript_options
},
})],
),
ModuleRule::new(
ModuleRuleCondition::ResourcePathEndsWith(".d.ts".to_string()),
Expand Down
9 changes: 4 additions & 5 deletions crates/turbopack/src/module_options/module_rule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,11 +99,10 @@ pub enum ModuleType {
},
Typescript {
transforms: Vc<EcmascriptInputTransforms>,
#[turbo_tasks(trace_ignore)]
options: EcmascriptOptions,
},
TypescriptWithTypes {
transforms: Vc<EcmascriptInputTransforms>,
// parse JSX syntax.
tsx: bool,
// follow references to imported types.
analyze_types: bool,
#[turbo_tasks(trace_ignore)]
options: EcmascriptOptions,
},
Expand Down

1 comment on commit 550126e

@vercel
Copy link

@vercel vercel bot commented on 550126e Jan 29, 2024

Choose a reason for hiding this comment

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

Please sign in to comment.