Skip to content

js_parser: keep this for a tagged template through a decorated #private member #42677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 1 commit into
base: main
Choose a base branch
from
Open

js_parser: keep this for a tagged template through a decorated #private member #42677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
80 changes: 57 additions & 23 deletions src/js_parser/lower/lower_decorators.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -328,27 +328,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
if let Some(info) = map.get(&pi.ref_.inner_index()).copied() {
let mut obj_expr = tgt_idx.target;
self.rewrite_private_accesses_in_expr(&mut obj_expr, map);
// `x.#m(...)` becomes `__privateGet(x, _m).call(x, ...)`, which
// references the receiver twice. Only identifiers and `this` can
// be repeated safely; any other receiver is captured in a
// temporary so its side effects run once and nested private
// calls don't duplicate the whole subtree (the duplication is
// exponential in the length of a chain like `o.#m().#m().#m()`).
let (get_obj, this_arg) = match &obj_expr.data {
js_ast::ExprData::EIdentifier(id) => {
let obj_ref = id.ref_;
(obj_expr, self.use_ref(obj_ref, obj_expr.loc))
}
js_ast::ExprData::EThis(_) => {
(obj_expr, self.new_expr(E::This {}, obj_expr.loc))
}
_ => {
let tmp_ref = self.declared_temp(b"_obj");
let write = self.assign_to(tmp_ref, obj_expr, expr_loc);
let read = self.use_ref(tmp_ref, expr_loc);
(write, read)
}
};
// `x.#m(...)` becomes `__privateGet(x, _m).call(x, ...)`.
let (get_obj, this_arg) = self.capture_receiver(obj_expr, expr_loc);
let private_access = self.private_get_expr(get_obj, &info, expr_loc);
let call_target = self.new_expr(
E::Dot {
Expand Down Expand Up @@ -421,8 +402,40 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}
}
js_ast::ExprData::ETemplate(e) => {
if let Some(t) = &mut e.tag {
self.rewrite_private_accesses_in_expr(t, map);
if let Some(tag) = &mut e.tag {
if let js_ast::ExprData::EIndex(tag_idx) = &tag.data
&& let js_ast::ExprData::EPrivateIdentifier(pi) = &tag_idx.index.data
&& let Some(info) = map.get(&pi.ref_.inner_index()).copied()
{
// ``x.#m`...` `` becomes ``__privateGet(x, _m).bind(x)`...` ``: the
// lowered tag is not a member access, so `this` has to be bound.
// `.call(x, strings)` cannot replace it, the strings array is the
// one the engine keeps for the call site.
let tag_loc = tag.loc;
let mut obj_expr = tag_idx.target;
self.rewrite_private_accesses_in_expr(&mut obj_expr, map);
let (get_obj, this_arg) = self.capture_receiver(obj_expr, tag_loc);
let private_access = self.private_get_expr(get_obj, &info, tag_loc);
let bind = self.new_expr(
E::Dot {
target: private_access,
name: b"bind".into(),
name_loc: tag_loc,
..Default::default()
},
tag_loc,
);
*tag = self.new_expr(
E::Call {
target: bind,
args: ExprNodeList::init_one(this_arg),
..Default::default()
},
tag_loc,
);
} else {
self.rewrite_private_accesses_in_expr(tag, map);
}
}
for part in e.parts_mut().iter_mut() {
self.rewrite_private_accesses_in_expr(&mut part.value, map);
Expand Down Expand Up @@ -517,6 +530,27 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O
}
}

/// Two references to the receiver of a lowered `x.#m(...)` or ``x.#m`...` ``:
/// the one that evaluates it, and the one that passes it as `this`. `this` and
/// an identifier are read twice. Any other receiver goes through a temporary,
/// so it is evaluated once and a chain like `o.#m().#m().#m()` does not
/// duplicate the whole subtree at each link.
fn capture_receiver(&mut self, obj: Expr, loc: bun_ast::Loc) -> (Expr, Expr) {
match &obj.data {
js_ast::ExprData::EIdentifier(id) => {
let obj_ref = id.ref_;
(obj, self.use_ref(obj_ref, obj.loc))
}
js_ast::ExprData::EThis(_) => (obj, self.new_expr(E::This {}, obj.loc)),
_ => {
let tmp_ref = self.declared_temp(b"_obj");
let write = self.assign_to(tmp_ref, obj, loc);
let read = self.use_ref(tmp_ref, loc);
(write, read)
}
}
}

/// Drain receiver-capture temporaries created past `baseline` into a
/// single `var` declaration statement; `None` if none were created.
fn drain_capture_temp_decls(&mut self, baseline: usize, loc: bun_ast::Loc) -> Option<Stmt> {
Expand Down
3 changes: 2 additions & 1 deletion src/jsc/RuntimeTranspilerCache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ bun_core::declare_scope!(cache, visible);
/// Version 30: String enum members are stored flat, so folds no longer append onto an inlined member.
/// Version 31: Standard decorator lowering temporaries have a per-file counter in their name (`_init$1`).
/// Version 32: Standard decorator lowering keeps class members in place.
const EXPECTED_VERSION: u32 = 32;
/// Version 33: Standard decorator lowering binds `this` for a tagged template whose tag is a lowered `#private` member.
const EXPECTED_VERSION: u32 = 33;

/// Source files smaller than this are not written to / read from the on-disk
/// transpiler cache. Originally 50 KiB, which excluded almost every file in a
Expand Down
71 changes: 71 additions & 0 deletions test/bundler/transpiler/es-decorators.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2188,6 +2188,57 @@ const extraSections = `
out.privateForms = [new SC().call(), new SC().swap()];
}

// A tagged template whose tag is a decorated \`#private\` member: \`this\` is the
// receiver, the receiver is evaluated once, and the strings array is the one
// of the call site. The expected values are what node prints for the same
// class without the decorators.
{
let n = 0;
const sites = [];
class PT {
@dec #tag(s, ...vals) { sites.push(s); return this?.n + ":" + s.raw.join("|") + ":" + vals.join(","); }
@dec static #stag(s, ...vals) { return (this === PT) + ":" + s.raw.join("|") + ":" + vals.join(","); }
@dec get #getter() { return function (s) { return this?.n + ":getter:" + s.raw[0]; }; }
@dec #field = function (s) { return this?.n + ":field:" + s.raw[0]; };
@dec accessor #acc = function (s) { return this?.n + ":accessor:" + s.raw[0]; };
@dec #me = this;
n = 7;
self() { n++; return this; }
static self() { n++; return PT; }
arrow = () => this.self().#tag\`arrow\`;
static { PT.fromBlock = PT.self().#stag\`block\`; }
run(other) {
return [
this.#tag\`a\${1}b\${2}\`,
PT.#stag\`x\${3}\`,
other.#tag\`o\`,
this.self().#tag\`c\`,
PT.self().#stag\`d\`,
this.#me.#tag\`m\`,
this.#getter\`e\`,
this.#field\`f\`,
this.#acc\`g\`,
this.#tag\`h\${this.self().#tag\`i\`}\`,
this.arrow(),
PT.fromBlock,
];
}
}
const pt = new PT();
const other = new PT();
other.n = 8;
const values = pt.run(other);
const perRun = sites.length;
pt.run(other);
out.privateTag = {
values,
receiverEvals: n,
sameSiteSameStrings: sites[0] === sites[perRun],
otherSiteOtherStrings: sites[0] !== sites[1],
raw: sites[0].raw,
};
}

// https://github.com/oven-sh/bun/issues/28118
{
const id = (value, context) => value;
Expand Down Expand Up @@ -2417,6 +2468,26 @@ const extraExpected = {
["hi:7/undefined/undefined", "sm:SC/g", "7:q|1"],
[10, 30],
],
privateTag: {
values: [
"7:a|b|:1,2",
"true:x|:3",
"8:o:",
"7:c:",
"true:d:",
"7:m:",
"7:getter:e",
"7:field:f",
"7:accessor:g",
"7:h|:7:i:",
"7:arrow:",
"true:block:",
],
receiverEvals: 9,
sameSiteSameStrings: true,
otherSiteOtherStrings: true,
raw: ["a", "b", ""],
},
issue28118: "hello",
issue31917: ["s", "t"],
issue31929: ["function", true],
Expand Down
Loading