Skip to content

feat(turbopack): externalType support promise#142

Merged
fireairforce merged 2 commits intoutoofrom
support-external-promise
Apr 28, 2026
Merged

feat(turbopack): externalType support promise#142
fireairforce merged 2 commits intoutoofrom
support-external-promise

Conversation

@fireairforce
Copy link
Copy Markdown
Member

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for the 'promise' external type within Turbopack. It adds the Promise variant to the ExternalType and CachedExternalType enums, updates the resolution logic, and ensures that these modules are correctly identified as asynchronous during code generation. A critical issue was identified in the code generation for promise externals: the current implementation incorrectly treats the request as a space-separated path to a global variable. To maintain compatibility with Webpack, the request should instead be treated as an arbitrary JavaScript expression and emitted directly.

Comment on lines +225 to +244
CachedExternalType::Promise => {
if self.request.is_empty() {
writeln!(code, "var mod = {kw} {{}};")?;
} else if self.request.contains(' ') {
let global_access = self
.request
.split(' ')
.fold("globalThis".to_string(), |acc, part| {
format!("{}[{}]", acc, StringifyJs(part))
});

writeln!(code, "var mod = {kw} {global_access};")?;
} else {
writeln!(
code,
"var mod = {kw} globalThis[{}];",
StringifyJs(&self.request)
)?;
}
}
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)?;
                }
            }

@fireairforce fireairforce merged commit 6f6135c into utoo Apr 28, 2026
14 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant