feat(turbopack): externalType support promise#142
Conversation
There was a problem hiding this comment.
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.
| 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) | ||
| )?; | ||
| } | ||
| } |
There was a problem hiding this comment.
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)?;
}
}
Support https://webpack.js.org/configuration/externals/#externalstypepromise