Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions turbopack/crates/turbopack-core/src/resolve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,7 @@ pub enum ExternalType {
CommonJs,
EcmaScriptModule,
Global,
Promise,
Script,
Umd,
}
Expand All @@ -489,6 +490,7 @@ impl Display for ExternalType {
ExternalType::EcmaScriptModule => write!(f, "esm"),
ExternalType::Url => write!(f, "url"),
ExternalType::Global => write!(f, "global"),
ExternalType::Promise => write!(f, "promise"),
ExternalType::Script => write!(f, "script"),
ExternalType::Umd => write!(f, "umd"),
}
Expand Down Expand Up @@ -3087,6 +3089,7 @@ async fn resolve_import_map_result(
ExternalType::Script
| ExternalType::Url
| ExternalType::Global
| ExternalType::Promise
| ExternalType::Umd => options,
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub enum CachedExternalType {
EcmaScriptViaRequire,
EcmaScriptViaImport,
Global,
Promise,
Script,
Umd,
}
Expand All @@ -74,6 +75,7 @@ impl Display for CachedExternalType {
CachedExternalType::EcmaScriptViaRequire => write!(f, "esm_require"),
CachedExternalType::EcmaScriptViaImport => write!(f, "esm_import"),
CachedExternalType::Global => write!(f, "global"),
CachedExternalType::Promise => write!(f, "promise"),
CachedExternalType::Script => write!(f, "script"),
CachedExternalType::Umd => write!(f, "umd"),
}
Expand Down Expand Up @@ -152,6 +154,7 @@ impl CachedExternalModule {
let mut code = RopeBuilder::default();

let needs_async_wrapper = self.external_type == CachedExternalType::EcmaScriptViaImport
|| self.external_type == CachedExternalType::Promise
|| self.external_type == CachedExternalType::Script;

// Use "yield" in legacy environments so the generator driver can step
Expand Down Expand Up @@ -219,6 +222,13 @@ impl CachedExternalModule {
)?;
}
}
CachedExternalType::Promise => {
if self.request.is_empty() {
writeln!(code, "var mod = {kw} {{}};")?;
} else {
writeln!(code, "var mod = {kw} ({});", self.request)?;
}
}
Comment on lines +225 to +231
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The current implementation treats the request for a promise external as a path to a global variable (splitting by space and accessing via globalThis). However, Webpack's externalsType: 'promise' (linked in the PR description) is defined to treat the value as an arbitrary JavaScript expression that returns a promise.

The current logic will break for common use cases like promise new Promise(...) or promise import(...) because it will attempt to split the expression by spaces and wrap it in globalThis[...]. For example, new Promise(...) would be incorrectly transformed into globalThis["new"]["Promise(...)"].

To match Webpack's behavior and support arbitrary expressions, the request should be emitted as a raw expression. This also simplifies the implementation by removing the redundant space-splitting and globalThis logic.

            CachedExternalType::Promise => {
                if self.request.is_empty() {
                    writeln!(code, "var mod = {kw} {{}};")?;
                } else {
                    writeln!(code, "var mod = {kw} ({});", self.request)?;
                }
            }

CachedExternalType::Umd => {
// request format is: "root React commonjs react"
let parts = self.request.split(' ').collect::<Vec<_>>();
Expand Down Expand Up @@ -358,19 +368,28 @@ fn externals_fs_root() -> Vc<FileSystemPath> {
impl Module for CachedExternalModule {
#[turbo_tasks::function]
async fn ident(&self) -> Result<Vc<AssetIdent>> {
// For script externals, simplify the path by using variable name
// instead of the full url to avoid long filenames
let path_str = if self.external_type == CachedExternalType::Script
&& let Some(at_index) = self.request.rfind('@').filter(|&i| i > 0)
{
self.request[..at_index].to_string()
} else {
self.request.to_string()
let (path_str, include_request_modifier) = match self.external_type {
CachedExternalType::Script => {
// Use the variable name instead of the full URL to avoid long filenames.
if let Some(at_index) = self.request.rfind('@').filter(|&i| i > 0) {
(self.request[..at_index].to_string(), true)
} else {
(self.request.to_string(), true)
}
}
CachedExternalType::Promise => {
let hash = encode_hex(hash_xxh3_hash64(self.request.as_str()));
(format!("promise-{}", &hash[..8]), false)
}
_ => (self.request.to_string(), true),
};
let mut ident = AssetIdent::from_path(externals_fs_root().await?.join(&path_str)?)
.with_layer(Layer::new(rcstr!("external")))
.with_modifier(self.request.clone())
.with_modifier(self.external_type.to_string().into());
.with_layer(Layer::new(rcstr!("external")));

if include_request_modifier {
ident = ident.with_modifier(self.request.clone());
}
ident = ident.with_modifier(self.external_type.to_string().into());

if let Some(target) = &self.target {
ident = ident.with_modifier(target.to_string_ref().await?);
Expand Down Expand Up @@ -412,6 +431,7 @@ impl Module for CachedExternalModule {
.await?
}
CachedExternalType::Global
| CachedExternalType::Promise
| CachedExternalType::Script
| CachedExternalType::Umd => {
origin
Expand Down Expand Up @@ -465,6 +485,7 @@ impl Module for CachedExternalModule {
fn is_self_async(&self) -> Result<Vc<bool>> {
Ok(Vc::cell(
self.external_type == CachedExternalType::EcmaScriptViaImport
|| self.external_type == CachedExternalType::Promise
|| self.external_type == CachedExternalType::Script,
))
}
Expand Down Expand Up @@ -502,6 +523,7 @@ impl EcmascriptChunkPlaceable for CachedExternalModule {
fn get_async_module(&self) -> Vc<OptionAsyncModule> {
Vc::cell(
if self.external_type == CachedExternalType::EcmaScriptViaImport
|| self.external_type == CachedExternalType::Promise
|| self.external_type == CachedExternalType::Script
{
Some(
Expand Down
1 change: 1 addition & 0 deletions turbopack/crates/turbopack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1252,6 +1252,7 @@ pub async fn replace_external(
}
}
ExternalType::Global => CachedExternalType::Global,
ExternalType::Promise => CachedExternalType::Promise,
ExternalType::Script => CachedExternalType::Script,
ExternalType::Umd => CachedExternalType::Umd,
ExternalType::Url => {
Expand Down
Loading