Skip to content

Commit a8077c3

Browse files
lukesandberglukesandberg
committed
Optimize re-export bindings
Fix an expr
1 parent 2604351 commit a8077c3

File tree

9 files changed

+294
-71
lines changed

9 files changed

+294
-71
lines changed

‎crates/next-core/src/next_dynamic/dynamic_module.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ impl EcmascriptChunkPlaceable for NextDynamicEntryModule {
105105
.to_resolved()
106106
.await?,
107107
);
108+
108109
let mut exports = BTreeMap::new();
109110
let default = rcstr!("default");
110111
exports.insert(

‎turbopack/crates/turbopack-ecmascript-runtime/js/src/shared/runtime-utils.ts‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,9 +105,12 @@ function createModuleObject(id: ModuleId): Module {
105105
}
106106
}
107107

108-
type BindingTag = 0
108+
type BindingTag = 0 | 1 | 2 | 3 | 4
109109
const BindingTag_Value = 0 as BindingTag
110-
110+
const BindingTag_ReExport = 1 as BindingTag
111+
const BindingTag_RenamedReexport = 2 as BindingTag
112+
const BindingTag_Mutable_ReExport = 3 as BindingTag
113+
const BindingTag_Mutable_RenamedReexport = 4 as BindingTag
111114
// an arbitrary sequence of bindings as
112115
// - a prop name
113116
// - BindingTag_Value, a value to be bound directly, or
@@ -127,15 +130,49 @@ function esm(exports: Exports, bindings: EsmBindings) {
127130
const propName = bindings[i++] as string
128131
const tagOrFunction = bindings[i++]
129132
if (typeof tagOrFunction === 'number') {
130-
if (tagOrFunction === BindingTag_Value) {
131-
defineProp(exports, propName, {
132-
value: bindings[i++],
133-
enumerable: true,
134-
writable: false,
135-
})
136-
} else {
137-
throw new Error(`unexpected tag: ${tagOrFunction}`)
133+
let descriptor: PropertyDescriptor
134+
switch (tagOrFunction) {
135+
case BindingTag_Value:
136+
descriptor = {
137+
value: bindings[i++],
138+
enumerable: true,
139+
writable: false,
140+
}
141+
break
142+
case BindingTag_ReExport:
143+
case BindingTag_Mutable_ReExport:
144+
{
145+
const namespace = bindings[i++] as Record<string, any>
146+
// Note: in the common case we can just copy the descriptor this removes some indirection when
147+
// accesing reexports however the propery may not exist if there is a cycle between a CJS file
148+
// and an ESM module, in that case we just bind a getter and optional setter.
149+
descriptor =
150+
Object.getOwnPropertyDescriptor(namespace, propName) ??
151+
makeReexportDescriptor(
152+
namespace,
153+
propName,
154+
tagOrFunction === BindingTag_Mutable_ReExport
155+
)
156+
}
157+
break
158+
case BindingTag_RenamedReexport:
159+
case BindingTag_Mutable_RenamedReexport:
160+
{
161+
const sourceName = bindings[i++] as string
162+
const namespace = bindings[i++] as Record<string, any>
163+
descriptor =
164+
Object.getOwnPropertyDescriptor(namespace, sourceName) ??
165+
makeReexportDescriptor(
166+
namespace,
167+
sourceName,
168+
tagOrFunction === BindingTag_Mutable_RenamedReexport
169+
)
170+
}
171+
break
172+
default:
173+
throw new Error(`unexpected tag: ${tagOrFunction}`)
138174
}
175+
defineProp(exports, propName, descriptor)
139176
} else {
140177
const getterFn = tagOrFunction as () => unknown
141178
if (typeof bindings[i] === 'function') {
@@ -156,6 +193,21 @@ function esm(exports: Exports, bindings: EsmBindings) {
156193
Object.seal(exports)
157194
}
158195

196+
function makeReexportDescriptor(
197+
namespace: Record<string, any>,
198+
propName: string,
199+
mutable: boolean
200+
): PropertyDescriptor {
201+
const descriptor: PropertyDescriptor = {
202+
enumerable: true,
203+
get: createGetter(namespace, propName),
204+
}
205+
if (mutable) {
206+
descriptor.set = createSetter(namespace, propName)
207+
}
208+
return descriptor
209+
}
210+
159211
/**
160212
* Makes the module an ESM with exports
161213
*/
@@ -275,7 +327,11 @@ contextPrototype.n = exportNamespace
275327
function createGetter(obj: Record<string | symbol, any>, key: string | symbol) {
276328
return () => obj[key]
277329
}
278-
330+
function createSetter(obj: Record<string | symbol, any>, key: string | symbol) {
331+
return (v) => {
332+
obj[key] = v
333+
}
334+
}
279335
/**
280336
* @returns prototype of the object
281337
*/

‎turbopack/crates/turbopack-ecmascript/src/references/esm/base.rs‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ use crate::{
4141
analyzer::imports::ImportAnnotations,
4242
chunk::{EcmascriptChunkPlaceable, EcmascriptExports},
4343
code_gen::{CodeGeneration, CodeGenerationHoistedStmt},
44+
export::Liveness,
4445
magic_identifier,
4546
references::{
4647
esm::EsmExport,
@@ -62,7 +63,11 @@ pub enum ReferencedAsset {
6263
#[derive(Debug)]
6364
pub enum ReferencedAssetIdent {
6465
/// The given export (or namespace) is a local binding in the current scope hoisting group.
65-
LocalBinding { ident: RcStr, ctxt: SyntaxContext },
66+
LocalBinding {
67+
ident: RcStr,
68+
ctxt: SyntaxContext,
69+
liveness: Liveness,
70+
},
6671
/// The given export (or namespace) should be imported and will be assigned to a new variable.
6772
Module {
6873
namespace_ident: String,
@@ -82,16 +87,33 @@ impl ReferencedAssetIdent {
8287
ReferencedAssetIdent::LocalBinding { .. } => None,
8388
}
8489
}
90+
pub fn as_module_namespace_expr(&self, span: Span) -> Option<Expr> {
91+
match self {
92+
ReferencedAssetIdent::Module {
93+
namespace_ident,
94+
ctxt,
95+
..
96+
} => Some(Expr::Ident(Ident::new(
97+
namespace_ident.as_str().into(),
98+
span,
99+
ctxt.unwrap_or_default(),
100+
))),
101+
ReferencedAssetIdent::LocalBinding { .. } => None,
102+
}
103+
}
85104

86105
pub fn as_expr_individual(&self, span: Span) -> Either<Ident, MemberExpr> {
87106
match self {
88-
ReferencedAssetIdent::LocalBinding { ident, ctxt } => {
89-
Either::Left(Ident::new(ident.as_str().into(), span, *ctxt))
90-
}
107+
ReferencedAssetIdent::LocalBinding {
108+
ident,
109+
ctxt,
110+
liveness: _,
111+
} => Either::Left(Ident::new(ident.as_str().into(), span, *ctxt)),
91112
ReferencedAssetIdent::Module {
92113
namespace_ident,
93114
ctxt,
94115
export,
116+
..
95117
} => {
96118
if let Some(export) = export {
97119
Either::Right(MemberExpr {
@@ -172,13 +194,14 @@ impl ReferencedAsset {
172194
let exports = exports.expand_exports(ModuleExportUsageInfo::all()).await?;
173195
let esm_export = exports.exports.get(export);
174196
match esm_export {
175-
Some(EsmExport::LocalBinding(_, _)) => {
197+
Some(EsmExport::LocalBinding(_, liveness)) => {
176198
// A local binding in a module that is merged in the same group. Use the
177199
// export name as identifier, it will be replaced with the actual
178200
// variable name during AST merging.
179201
return Ok(Some(ReferencedAssetIdent::LocalBinding {
180202
ident: export.clone(),
181203
ctxt,
204+
liveness: *liveness,
182205
}));
183206
}
184207
Some(b @ EsmExport::ImportedBinding(esm_ref, _, _))

‎turbopack/crates/turbopack-ecmascript/src/references/esm/export.rs‎

Lines changed: 87 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,8 @@ impl EsmExports {
649649
Getter(Expr),
650650
GetterSetter(Expr, Expr),
651651
Value(Expr),
652+
// The namespace expression and the export name
653+
ReExport(Expr, bool, RcStr),
652654
None,
653655
}
654656

@@ -686,9 +688,7 @@ impl EsmExports {
686688

687689
let local = Ident::new(local.into(), DUMMY_SP, ctxt);
688690
match (liveness, export_usage_info.is_circuit_breaker) {
689-
(Liveness::Constant, false) => {
690-
ExportBinding::Value(quote!("$local" as Expr, local = local))
691-
}
691+
(Liveness::Constant, false) => ExportBinding::Value(Expr::Ident(local)),
692692
// If the value might change or we are a circuit breaker we must bind a
693693
// getter to avoid capturing the value at the wrong time.
694694
(Liveness::Live, _) | (Liveness::Constant, true) => {
@@ -713,31 +713,67 @@ impl EsmExports {
713713
.map(|ident| {
714714
let expr = ident.as_expr_individual(DUMMY_SP);
715715
let read_expr = expr.map_either(Expr::from, Expr::from).into_inner();
716-
// For imported bindings we could simply export the 'import' and have the runtime hook the bindings together.
717-
// This would be cute, slightly smaller codegen but efficiency is an open question. In fact we could even
718-
// perform the 'liveness' check at runtime by querying the property descriptor.
719-
let getter = quote!(
720-
"() => $expr" as Expr,
721-
expr: Expr = read_expr,
722-
);
723-
if *mutable {
724-
ExportBinding::GetterSetter(
725-
getter,
726-
quote!(
727-
"($new) => $lhs = $new" as Expr,
728-
lhs: AssignTarget = AssignTarget::Simple(
729-
ident.as_expr_individual(DUMMY_SP).map_either(|i| SimpleAssignTarget::Ident(i.into()), SimpleAssignTarget::Member).into_inner()),
730-
new = Ident::new(
731-
format!("new_{name}").into(),
732-
DUMMY_SP,
733-
Default::default()
734-
),
735-
)
736-
)
737-
} else {
738-
ExportBinding::Getter(getter)
739-
}
740-
}).unwrap_or(ExportBinding::None)
716+
use crate::references::esm::base::ReferencedAssetIdent;
717+
match &ident {
718+
ReferencedAssetIdent::LocalBinding {ctxt, liveness,.. } => {
719+
debug_assert!(*mutable == (*liveness == Liveness::Mutable), "If the re-export is mutable, the merged local must be too");
720+
// If we are re-exporting something but got merged with it we can treat it like a local export
721+
match (liveness, export_usage_info.is_circuit_breaker) {
722+
(Liveness::Constant, false) => {
723+
ExportBinding::Value(read_expr)
724+
}
725+
// If the value might change or we are a circuit breaker we must bind a
726+
// getter to avoid capturing the value at the wrong time.
727+
(Liveness::Live, _) | (Liveness::Constant, true) => {
728+
ExportBinding::Getter(quote!("() => $local" as Expr, local: Expr = read_expr))
729+
}
730+
(Liveness::Mutable, _) => ExportBinding::GetterSetter(
731+
quote!("() => $local" as Expr, local: Expr= read_expr.clone()),
732+
quote!(
733+
"($new) => $lhs = $new" as Expr,
734+
lhs: AssignTarget = AssignTarget::Simple(
735+
ident.as_expr_individual(DUMMY_SP).map_either(|i| SimpleAssignTarget::Ident(i.into()), SimpleAssignTarget::Member).into_inner()),
736+
new = Ident::new(format!("new_{name}").into(), DUMMY_SP, *ctxt),
737+
),
738+
),
739+
}
740+
741+
},
742+
ReferencedAssetIdent::Module { namespace_ident:_, ctxt:_, export } => {
743+
744+
if export_usage_info.is_circuit_breaker {
745+
// Instead of a function we could pass a module-id up and have the runtime bind a getter that retrieves it from the modulecache
746+
let getter = quote!(
747+
"() => $expr" as Expr,
748+
expr: Expr = read_expr,
749+
);
750+
if *mutable {
751+
ExportBinding::GetterSetter(
752+
getter,
753+
quote!(
754+
"($new) => $lhs = $new" as Expr,
755+
lhs: AssignTarget = AssignTarget::Simple(
756+
ident.as_expr_individual(DUMMY_SP).map_either(|i| SimpleAssignTarget::Ident(i.into()), SimpleAssignTarget::Member).into_inner()),
757+
new = Ident::new(
758+
format!("new_{name}").into(),
759+
DUMMY_SP,
760+
Default::default()
761+
),
762+
))
763+
} else {
764+
ExportBinding::Getter(getter)
765+
}
766+
} else {
767+
let namespace_expr = ident.as_module_namespace_expr(DUMMY_SP).unwrap();
768+
if let Some(export) = export {
769+
ExportBinding::ReExport(namespace_expr, *mutable, export.clone())
770+
} else {
771+
ExportBinding::Value(namespace_expr)
772+
}
773+
}
774+
}
775+
}})
776+
.unwrap_or(ExportBinding::None)
741777
}
742778
EsmExport::ImportedNamespace(esm_ref) => {
743779
let referenced_asset =
@@ -748,6 +784,10 @@ impl EsmExports {
748784
.map(|ident| {
749785
let imported = ident.as_expr(DUMMY_SP, false);
750786
if export_usage_info.is_circuit_breaker {
787+
// In theory we could just pass the target module id up and the
788+
// runtime could do the import when binding the export.
789+
// This would almost work but could change evaluation order unless
790+
// we create some way to look-up a module without loading it.
751791
ExportBinding::Getter(quote!(
752792
"(() => $imported)" as Expr,
753793
imported: Expr = imported
@@ -783,6 +823,25 @@ impl EsmExports {
783823
getters.push(Some(value.into()));
784824
}
785825
ExportBinding::None => {}
826+
ExportBinding::ReExport(namespace, mutable, export) => {
827+
let offset = if mutable { 2 } else { 0 };
828+
if &export == exported {
829+
getters
830+
.push(Some(Expr::Lit(Lit::Num(Number::from(1 + offset))).into()));
831+
} else {
832+
getters
833+
.push(Some(Expr::Lit(Lit::Num(Number::from(2 + offset))).into()));
834+
getters.push(Some(
835+
Expr::Lit(Lit::Str(Str {
836+
span: DUMMY_SP,
837+
value: export.as_str().into(),
838+
raw: None,
839+
}))
840+
.into(),
841+
))
842+
}
843+
getters.push(Some(namespace.into()));
844+
}
786845
};
787846
}
788847
}

‎turbopack/crates/turbopack-tests/tests/snapshot/intermediate-tree-shake/rename-side-effect-free-facade/output/53446_snapshot_intermediate-tree-shake_rename-side-effect-free-facade_input_cbcbaae7._.js‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)