From 9b632810ae1c56ab44e9ea2928bf9de1d159b91e Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Tue, 30 Jul 2024 16:49:40 +0200 Subject: [PATCH] Replace MDX ModuleType with MDX SourceTransform (vercel/turbo#8766) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Description Remove `ModuleType::Mdx` and instead handle mdx files using ```rust ModuleRuleEffect::ModuleType(ModuleType::Typescript { transforms: ts_app_transforms, tsx: true, analyze_types: enable_types, options: ecmascript_options_vc, }), ModuleRuleEffect::SourceTransforms(Vc::cell(vec![Vc::upcast( MdxTransform::new(mdx_transform_options), )])), ``` ### Testing Instructions I ran these Next.js tests, which appear to be all mdx related tests there are ``` mdx with-mdx-rs app directory ✓ should work in initial html (1262 ms) ✓ should work using browser (1460 ms) ✓ should work in initial html with mdx import (145 ms) ✓ should work using browser with mdx import (1179 ms) ✓ should allow overriding components (1163 ms) ✓ should allow importing client components (26 ms) ✓ should work with next/image (424 ms) pages directory ✓ should work in initial html (1091 ms) ✓ should work using browser (1216 ms) ✓ should work in initial html with mdx import (147 ms) ✓ should work using browser with mdx import (1202 ms) ✓ should allow overriding components (1236 ms) ``` --- crates/turbopack-mdx/src/lib.rs | 320 +++++------------- crates/turbopack/src/lib.rs | 21 -- crates/turbopack/src/module_options/mod.rs | 25 +- .../src/module_options/module_rule.rs | 9 - 4 files changed, 95 insertions(+), 280 deletions(-) diff --git a/crates/turbopack-mdx/src/lib.rs b/crates/turbopack-mdx/src/lib.rs index 9468495d4a9bc..a60d691efe8ba 100644 --- a/crates/turbopack-mdx/src/lib.rs +++ b/crates/turbopack-mdx/src/lib.rs @@ -1,29 +1,16 @@ #![feature(min_specialization)] #![feature(arbitrary_self_types)] -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, Result}; use mdxjs::{compile, MdxParseOptions, Options}; -use turbo_tasks::{RcStr, Value, ValueDefault, Vc}; -use turbo_tasks_fs::{rope::Rope, File, FileContent, FileSystemPath}; +use turbo_tasks::{RcStr, ValueDefault, Vc}; +use turbo_tasks_fs::{rope::Rope, File, FileContent}; use turbopack_core::{ asset::{Asset, AssetContent}, - chunk::{AsyncModuleInfo, ChunkItem, ChunkType, ChunkableModule, ChunkingContext}, - context::AssetContext, ident::AssetIdent, - module::Module, - reference::ModuleReferences, - resolve::origin::ResolveOrigin, + issue::IssueDescriptionExt, source::Source, - virtual_source::VirtualSource, -}; -use turbopack_ecmascript::{ - chunk::{ - EcmascriptChunkItem, EcmascriptChunkItemContent, EcmascriptChunkPlaceable, - EcmascriptChunkType, EcmascriptExports, - }, - references::AnalyzeEcmascriptModuleResultBuilder, - AnalyzeEcmascriptModuleResult, EcmascriptInputTransforms, EcmascriptModuleAsset, - EcmascriptModuleAssetType, EcmascriptOptions, + source_transform::SourceTransform, }; #[turbo_tasks::function] @@ -86,247 +73,122 @@ impl ValueDefault for MdxTransformOptions { } #[turbo_tasks::value] -#[derive(Clone, Copy)] -pub struct MdxModuleAsset { - source: Vc>, - asset_context: Vc>, - transforms: Vc, +pub struct MdxTransform { options: Vc, - ecmascript_options: Vc, -} - -/// MDX components should be treated as normal j|tsx components to analyze -/// its imports or run ecma transforms, -/// only difference is it is not a valid ecmascript AST we -/// can't pass it forward directly. Internally creates an jsx from mdx -/// via mdxrs, then pass it through existing ecmascript analyzer. -/// -/// To make mdx as a variant of ecmascript and use its `source_transforms` -/// instead, there should be a way to get a valid SWC ast from mdx source input -/// - which we don't have yet. -async fn into_ecmascript_module_asset( - current_context: &Vc, -) -> Result> { - let content = current_context.content(); - let this = current_context.await?; - let transform_options = this.options.await?; - - let AssetContent::File(file) = &*content.await? else { - anyhow::bail!("Unexpected mdx asset content"); - }; - - let FileContent::Content(file) = &*file.await? else { - anyhow::bail!("Not able to read mdx file content"); - }; - - let jsx_runtime = if let Some(runtime) = &transform_options.jsx_runtime { - match runtime.as_str() { - "automatic" => Some(mdxjs::JsxRuntime::Automatic), - "classic" => Some(mdxjs::JsxRuntime::Classic), - _ => None, - } - } else { - None - }; - - let parse_options = match transform_options.mdx_type { - Some(MdxParseConstructs::Gfm) => MdxParseOptions::gfm(), - _ => MdxParseOptions::default(), - }; - - let options = Options { - parse: parse_options, - development: transform_options.development.unwrap_or(false), - provider_import_source: transform_options - .provider_import_source - .clone() - .map(RcStr::into_owned), - jsx: transform_options.jsx.unwrap_or(false), // true means 'preserve' jsx syntax. - jsx_runtime, - jsx_import_source: transform_options - .jsx_import_source - .clone() - .map(RcStr::into_owned), - filepath: Some(this.source.ident().path().await?.to_string()), - ..Default::default() - }; - // TODO: upstream mdx currently bubbles error as string - let mdx_jsx_component = - compile(&file.content().to_str()?, &options).map_err(|e| anyhow!("{}", e))?; - - let source = VirtualSource::new_with_ident( - this.source.ident(), - AssetContent::file(File::from(Rope::from(mdx_jsx_component)).into()), - ); - Ok(EcmascriptModuleAsset::new( - Vc::upcast(source), - this.asset_context, - Value::new(EcmascriptModuleAssetType::Typescript { - tsx: true, - analyze_types: false, - }), - this.transforms, - this.ecmascript_options, - this.asset_context.compile_time_info(), - )) } #[turbo_tasks::value_impl] -impl MdxModuleAsset { - #[turbo_tasks::function] - pub fn new( - source: Vc>, - asset_context: Vc>, - transforms: Vc, - options: Vc, - ecmascript_options: Vc, - ) -> Vc { - Self::cell(MdxModuleAsset { - source, - asset_context, - transforms, - options, - ecmascript_options, - }) - } - +impl MdxTransform { #[turbo_tasks::function] - async fn analyze(self: Vc) -> Result> { - let asset = into_ecmascript_module_asset(&self).await; - - if let Ok(asset) = asset { - Ok(asset.analyze()) - } else { - let mut result = AnalyzeEcmascriptModuleResultBuilder::new(); - result.set_successful(false); - result.build(false).await - } + pub fn new(options: Vc) -> Vc { + MdxTransform { options }.cell() } } #[turbo_tasks::value_impl] -impl Module for MdxModuleAsset { +impl SourceTransform for MdxTransform { #[turbo_tasks::function] - fn ident(&self) -> Vc { - self.source - .ident() - .with_modifier(modifier()) - .with_layer(self.asset_context.layer()) - } - - #[turbo_tasks::function] - async fn references(self: Vc) -> Result> { - let analyze = self.analyze().await?; - Ok(analyze.references) + fn transform(&self, source: Vc>) -> Vc> { + Vc::upcast( + MdxTransformedAsset { + options: self.options, + source, + } + .cell(), + ) } } -#[turbo_tasks::value_impl] -impl Asset for MdxModuleAsset { - #[turbo_tasks::function] - fn content(&self) -> Vc { - self.source.content() - } +#[turbo_tasks::value] +struct MdxTransformedAsset { + options: Vc, + source: Vc>, } #[turbo_tasks::value_impl] -impl ChunkableModule for MdxModuleAsset { +impl Source for MdxTransformedAsset { #[turbo_tasks::function] - async fn as_chunk_item( - self: Vc, - chunking_context: Vc>, - ) -> Result>> { - Ok(Vc::upcast(MdxChunkItem::cell(MdxChunkItem { - module: self, - chunking_context, - }))) + fn ident(&self) -> Vc { + self.source.ident().rename_as("*.tsx".into()) } } #[turbo_tasks::value_impl] -impl EcmascriptChunkPlaceable for MdxModuleAsset { +impl Asset for MdxTransformedAsset { #[turbo_tasks::function] - fn get_exports(&self) -> Vc { - EcmascriptExports::Value.cell() + async fn content(self: Vc) -> Result> { + let this = self.await?; + Ok(self + .process() + .issue_file_path(this.source.ident().path(), "MDX processing") + .await? + .await? + .content) } } #[turbo_tasks::value_impl] -impl ResolveOrigin for MdxModuleAsset { - #[turbo_tasks::function] - fn origin_path(&self) -> Vc { - self.source.ident().path() - } - - #[turbo_tasks::function] - fn asset_context(&self) -> Vc> { - self.asset_context +impl MdxTransformedAsset { + #[turbo_tasks::function] + async fn process(self: Vc) -> Result> { + let this = self.await?; + let content = this.source.content().await?; + let transform_options = this.options.await?; + + let AssetContent::File(file) = &*content else { + anyhow::bail!("Unexpected mdx asset content"); + }; + + let FileContent::Content(file) = &*file.await? else { + anyhow::bail!("Not able to read mdx file content"); + }; + + let jsx_runtime = if let Some(runtime) = &transform_options.jsx_runtime { + match runtime.as_str() { + "automatic" => Some(mdxjs::JsxRuntime::Automatic), + "classic" => Some(mdxjs::JsxRuntime::Classic), + _ => None, + } + } else { + None + }; + + let parse_options = match transform_options.mdx_type { + Some(MdxParseConstructs::Gfm) => MdxParseOptions::gfm(), + _ => MdxParseOptions::default(), + }; + + let options = Options { + parse: parse_options, + development: transform_options.development.unwrap_or(false), + provider_import_source: transform_options + .provider_import_source + .clone() + .map(RcStr::into_owned), + jsx: transform_options.jsx.unwrap_or(false), // true means 'preserve' jsx syntax. + jsx_runtime, + jsx_import_source: transform_options + .jsx_import_source + .clone() + .map(RcStr::into_owned), + filepath: Some(this.source.ident().path().await?.to_string()), + ..Default::default() + }; + + // TODO: upstream mdx currently bubbles error as string + let mdx_jsx_component = + compile(&file.content().to_str()?, &options).map_err(|e| anyhow!("{}", e))?; + + Ok(MdxTransformResult { + content: AssetContent::file(File::from(Rope::from(mdx_jsx_component)).into()), + } + .cell()) } } #[turbo_tasks::value] -struct MdxChunkItem { - module: Vc, - chunking_context: Vc>, -} - -#[turbo_tasks::value_impl] -impl ChunkItem for MdxChunkItem { - #[turbo_tasks::function] - fn asset_ident(&self) -> Vc { - self.module.ident() - } - - #[turbo_tasks::function] - fn references(&self) -> Vc { - self.module.references() - } - - #[turbo_tasks::function] - async fn chunking_context(&self) -> Vc> { - Vc::upcast(self.chunking_context) - } - - #[turbo_tasks::function] - async fn ty(&self) -> Result>> { - Ok(Vc::upcast( - Vc::::default().resolve().await?, - )) - } - - #[turbo_tasks::function] - fn module(&self) -> Vc> { - Vc::upcast(self.module) - } -} - -#[turbo_tasks::value_impl] -impl EcmascriptChunkItem for MdxChunkItem { - #[turbo_tasks::function] - fn chunking_context(&self) -> Vc> { - self.chunking_context - } - - #[turbo_tasks::function] - fn content(self: Vc) -> Vc { - panic!("MdxChunkItem::content should never be called"); - } - - /// Once we have mdx contents, we should treat it as j|tsx components and - /// apply all of the ecma transforms - #[turbo_tasks::function] - async fn content_with_async_module_info( - &self, - async_module_info: Option>, - ) -> Result> { - let item = into_ecmascript_module_asset(&self.module) - .await? - .as_chunk_item(Vc::upcast(self.chunking_context)); - let ecmascript_item = Vc::try_resolve_downcast::>(item) - .await? - .context("MdxChunkItem must generate an EcmascriptChunkItem")?; - Ok(ecmascript_item.content_with_async_module_info(async_module_info)) - } +struct MdxTransformResult { + content: Vc, } pub fn register() { diff --git a/crates/turbopack/src/lib.rs b/crates/turbopack/src/lib.rs index 10449ebdabc40..f17d91d11ac9a 100644 --- a/crates/turbopack/src/lib.rs +++ b/crates/turbopack/src/lib.rs @@ -56,7 +56,6 @@ pub use turbopack_css as css; pub use turbopack_ecmascript as ecmascript; use turbopack_ecmascript::references::external_module::{CachedExternalModule, CachedExternalType}; use turbopack_json::JsonModuleAsset; -use turbopack_mdx::MdxModuleAsset; pub use turbopack_resolve::{resolve::resolve_options, resolve_options_context}; use turbopack_resolve::{resolve_options_context::ResolveOptionsContext, typescript::type_resolve}; use turbopack_static::StaticModuleAsset; @@ -277,17 +276,6 @@ async fn apply_module_type( source, Vc::upcast(module_asset_context), )), - ModuleType::Mdx { - transforms, - options, - ecmascript_options, - } => Vc::upcast(MdxModuleAsset::new( - source, - Vc::upcast(module_asset_context), - *transforms, - *options, - *ecmascript_options, - )), ModuleType::WebAssembly { source_ty } => Vc::upcast(WebAssemblyModuleAsset::new( WebAssemblySource::new(source, *source_ty), Vc::upcast(module_asset_context), @@ -538,15 +526,6 @@ async fn process_default_internal( analyze_types, options, }), - Some(ModuleType::Mdx { - transforms, - options, - ecmascript_options, - }) => Some(ModuleType::Mdx { - transforms: prepend.extend(transforms).extend(*append), - options, - ecmascript_options, - }), Some(module_type) => { ModuleIssue { ident, diff --git a/crates/turbopack/src/module_options/mod.rs b/crates/turbopack/src/module_options/mod.rs index d6c6e1ed313cb..c299b08d3c1d7 100644 --- a/crates/turbopack/src/module_options/mod.rs +++ b/crates/turbopack/src/module_options/mod.rs @@ -16,6 +16,7 @@ use turbopack_core::{ }; use turbopack_css::CssModuleAssetType; use turbopack_ecmascript::{EcmascriptInputTransform, EcmascriptOptions, SpecifiedModuleType}; +use turbopack_mdx::MdxTransform; use turbopack_node::transforms::{postcss::PostCssTransform, webpack::WebpackLoaders}; use turbopack_wasm::source::WebAssemblySourceType; @@ -186,22 +187,6 @@ impl ModuleOptions { Vc::cell(transforms.clone()) }; - let mdx_transforms = Vc::cell( - if let Some(transform) = &ts_transform { - if let Some(decorators_transform) = &decorators_transform { - vec![decorators_transform.clone(), transform.clone()] - } else { - vec![transform.clone()] - } - } else { - vec![] - } - .iter() - .cloned() - .chain(transforms.iter().cloned()) - .collect(), - ); - // Apply decorators transform for the ModuleType::Ecmascript as well after // constructing ts_app_transforms. Ecmascript can have decorators for // the cases of 1. using jsconfig, to enable ts-specific runtime @@ -535,11 +520,9 @@ impl ModuleOptions { ModuleRuleCondition::ResourcePathEndsWith(".md".to_string()), ModuleRuleCondition::ResourcePathEndsWith(".mdx".to_string()), ]), - vec![ModuleRuleEffect::ModuleType(ModuleType::Mdx { - transforms: mdx_transforms, - options: mdx_transform_options, - ecmascript_options: ecmascript_options_vc, - })], + vec![ModuleRuleEffect::SourceTransforms(Vc::cell(vec![ + Vc::upcast(MdxTransform::new(mdx_transform_options)), + ]))], )); } diff --git a/crates/turbopack/src/module_options/module_rule.rs b/crates/turbopack/src/module_options/module_rule.rs index 710be28eaacad..6db0f92bf8cc4 100644 --- a/crates/turbopack/src/module_options/module_rule.rs +++ b/crates/turbopack/src/module_options/module_rule.rs @@ -7,7 +7,6 @@ use turbopack_core::{ }; use turbopack_css::CssModuleAssetType; use turbopack_ecmascript::{EcmascriptInputTransforms, EcmascriptOptions}; -use turbopack_mdx::MdxTransformOptions; use turbopack_wasm::source::WebAssemblySourceType; use super::{CustomModuleType, ModuleRuleCondition}; @@ -119,14 +118,6 @@ pub enum ModuleType { }, Json, Raw, - // [TODO] We want to consolidate mdx as a type of ecma|typescript module types with - // its source transform. Refer `turbopack-mdx::into_ecmascript_module_asset` for the reason - // why we keep this separately. - Mdx { - transforms: Vc, - options: Vc, - ecmascript_options: Vc, - }, CssGlobal, CssModule, Css {