Skip to content

feat(minifier): allow compressing computed constructor/prototype keys precisely #9594

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

Merged
graphite-app merged 1 commit into from
Mar 7, 2025
Merged

feat(minifier): allow compressing computed constructor/prototype keys precisely #9594

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
114 changes: 108 additions & 6 deletions crates/oxc_minifier/src/peephole/substitute_alternate_syntax.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ impl<'a> PeepholeOptimizations {
prop: &mut MethodDefinition<'a>,
ctx: Ctx<'a, '_>,
) {
let property_key_parent: ClassPropertyKeyParent = prop.into();
if let PropertyKey::StringLiteral(str) = &prop.key {
if property_key_parent.should_keep_as_computed_property(&str.value) {
return;
}
}
self.try_compress_property_key(&mut prop.key, &mut prop.computed, ctx);
}

Expand All @@ -86,6 +92,12 @@ impl<'a> PeepholeOptimizations {
prop: &mut PropertyDefinition<'a>,
ctx: Ctx<'a, '_>,
) {
let property_key_parent: ClassPropertyKeyParent = prop.into();
if let PropertyKey::StringLiteral(str) = &prop.key {
if property_key_parent.should_keep_as_computed_property(&str.value) {
return;
}
}
self.try_compress_property_key(&mut prop.key, &mut prop.computed, ctx);
}

Expand All @@ -94,6 +106,12 @@ impl<'a> PeepholeOptimizations {
prop: &mut AccessorProperty<'a>,
ctx: Ctx<'a, '_>,
) {
let property_key_parent: ClassPropertyKeyParent = prop.into();
if let PropertyKey::StringLiteral(str) = &prop.key {
if property_key_parent.should_keep_as_computed_property(&str.value) {
return;
}
}
self.try_compress_property_key(&mut prop.key, &mut prop.computed, ctx);
}

Expand Down Expand Up @@ -847,9 +865,7 @@ impl<'a> PeepholeOptimizations {
};
let PropertyKey::StringLiteral(s) = key else { return };
let value = s.value.as_str();
// Uncaught SyntaxError: Classes may not have a field named 'constructor'
// Uncaught SyntaxError: Class constructor may not be a private method
if matches!(value, "__proto__" | "prototype" | "constructor" | "#constructor") {
if value == "__proto__" {
return;
}
if is_identifier_name(value) {
Expand Down Expand Up @@ -1107,6 +1123,68 @@ impl<'a> LatePeepholeOptimizations {
}
}

struct ClassPropertyKeyParent {
pub ty: ClassPropertyKeyParentType,
/// Whether the property is static.
pub r#static: bool,
}

impl ClassPropertyKeyParent {
/// Whether the key should be kept as a computed property to avoid early errors.
///
/// <https://tc39.es/ecma262/2024/multipage/ecmascript-language-functions-and-classes.html#sec-static-semantics-classelementkind>
/// <https://tc39.es/ecma262/2024/multipage/ecmascript-language-functions-and-classes.html#sec-class-definitions-static-semantics-early-errors>
/// <https://arai-a.github.io/ecma262-compare/?pr=2417&id=sec-class-definitions-static-semantics-early-errors>
fn should_keep_as_computed_property(&self, key: &str) -> bool {
match key {
"prototype" => self.r#static,
"constructor" => match self.ty {
// Uncaught SyntaxError: Class constructor may not be an accessor
ClassPropertyKeyParentType::MethodDefinition => !self.r#static,
// Uncaught SyntaxError: Classes may not have a field named 'constructor'
// Uncaught SyntaxError: Class constructor may not be a private method
ClassPropertyKeyParentType::AccessorProperty
| ClassPropertyKeyParentType::PropertyDefinition => true,
},
"#constructor" => true,
_ => false,
}
}
}

enum ClassPropertyKeyParentType {
PropertyDefinition,
AccessorProperty,
MethodDefinition,
}

impl From<&PropertyDefinition<'_>> for ClassPropertyKeyParent {
fn from(prop: &PropertyDefinition<'_>) -> Self {
Self { ty: ClassPropertyKeyParentType::PropertyDefinition, r#static: prop.r#static }
}
}

impl From<&AccessorProperty<'_>> for ClassPropertyKeyParent {
fn from(accessor: &AccessorProperty<'_>) -> Self {
Self { ty: ClassPropertyKeyParentType::AccessorProperty, r#static: accessor.r#static }
}
}

impl From<&MethodDefinition<'_>> for ClassPropertyKeyParent {
fn from(method: &MethodDefinition<'_>) -> Self {
Self { ty: ClassPropertyKeyParentType::MethodDefinition, r#static: method.r#static }
}
}

impl<T> From<&mut T> for ClassPropertyKeyParent
where
ClassPropertyKeyParent: for<'a> std::convert::From<&'a T>,
{
fn from(prop: &mut T) -> Self {
(&*prop).into()
}
}

/// Port from <https://github.com/google/closure-compiler/blob/v20240609/test/com/google/javascript/jscomp/PeepholeSubstituteAlternateSyntaxTest.java>
#[cfg(test)]
mod test {
Expand Down Expand Up @@ -1607,10 +1685,34 @@ mod test {
);

test("class C { ['-1']() {} }", "class C { '-1'() {} }");
test_same("class C { ['prototype']() {} }");
test_same("class C { ['__proto__']() {} }");
test_same("class C { ['constructor']() {} }");
test_same("class C { ['#constructor']() {} }");

// <https://tc39.es/ecma262/2024/multipage/ecmascript-language-functions-and-classes.html#sec-static-semantics-classelementkind>
// <https://tc39.es/ecma262/2024/multipage/ecmascript-language-functions-and-classes.html#sec-class-definitions-static-semantics-early-errors>
// <https://arai-a.github.io/ecma262-compare/?pr=2417&id=sec-class-definitions-static-semantics-early-errors>
test_same("class C { static ['prototype']() {} }"); // class C { static prototype() {} } is an early error
test_same("class C { static ['prototype'] = 0 }"); // class C { prototype = 0 } is an early error
test_same("class C { static accessor ['prototype'] = 0 }"); // class C { accessor prototype = 0 } is an early error
test("class C { ['prototype']() {} }", "class C { prototype() {} }");
test("class C { ['prototype'] = 0 }", "class C { prototype = 0 }");
test("class C { accessor ['prototype'] = 0 }", "class C { accessor prototype = 0 }");
test_same("class C { ['constructor'] = 0 }"); // class C { constructor = 0 } is an early error
test_same("class C { accessor ['constructor'] = 0 }"); // class C { accessor constructor = 0 } is an early error
test_same("class C { static ['constructor'] = 0 }"); // class C { static constructor = 0 } is an early error
test_same("class C { static accessor ['constructor'] = 0 }"); // class C { static accessor constructor = 0 } is an early error
test_same("class C { ['constructor']() {} }"); // computed `constructor` is not treated as a constructor
test_same("class C { *['constructor']() {} }"); // class C { *constructor() {} } is an early error
test_same("class C { async ['constructor']() {} }"); // class C { async constructor() {} } is an early error
test_same("class C { async *['constructor']() {} }"); // class C { async *constructor() {} } is an early error
test_same("class C { get ['constructor']() {} }"); // class C { get constructor() {} } is an early error
test_same("class C { set ['constructor'](v) {} }"); // class C { set constructor(v) {} } is an early error
test("class C { static ['constructor']() {} }", "class C { static constructor() {} }");
test_same("class C { ['#constructor'] = 0 }"); // class C { #constructor = 0 } is an early error
test_same("class C { accessor ['#constructor'] = 0 }"); // class C { accessor #constructor = 0 } is an early error
test_same("class C { ['#constructor']() {} }"); // class C { #constructor() {} } is an early error
test_same("class C { static ['#constructor'] = 0 }"); // class C { static #constructor = 0 } is an early error
test_same("class C { static accessor ['#constructor'] = 0 }"); // class C { static accessor #constructor = 0 } is an early error
test_same("class C { static ['#constructor']() {} }"); // class C { static #constructor() {} } is an early error
}

#[test]
Expand Down
100 changes: 50 additions & 50 deletions crates/oxc_minifier/tests/peephole/esbuild.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,56 +70,56 @@ fn js_parser_test() {
"function f() { x() } var f; function f() { y() }",
"function f() { x();}var f;function f() { y();}",
);
// test("class Foo { ['constructor'] = 0 }", "class Foo { ['constructor'] = 0;}");
// test("class Foo { ['constructor']() {} }", "class Foo { ['constructor']() { }}");
// test("class Foo { *['constructor']() {} }", "class Foo { *['constructor']() { }}");
// test("class Foo { get ['constructor']() {} }", "class Foo { get ['constructor']() { }}");
// test("class Foo { set ['constructor'](x) {} }", "class Foo { set ['constructor'](x) { }}");
// test("class Foo { async ['constructor']() {} }", "class Foo { async ['constructor']() { }}");
// test("class Foo { static ['constructor'] = 0 }", "class Foo { static ['constructor'] = 0;}");
// test("class Foo { static ['constructor']() {} }", "class Foo { static constructor() { }}");
// test("class Foo { static *['constructor']() {} }", "class Foo { static *constructor() { }}");
// test(
// "class Foo { static get ['constructor']() {} }",
// "class Foo { static get constructor() { }}",
// );
// test(
// "class Foo { static set ['constructor'](x) {} }",
// "class Foo { static set constructor(x) { }}",
// );
// test(
// "class Foo { static async ['constructor']() {} }",
// "class Foo { static async constructor() { }}",
// );
// test("class Foo { ['prototype'] = 0 }", "class Foo { prototype = 0;}");
// test("class Foo { ['prototype']() {} }", "class Foo { prototype() { }}");
// test("class Foo { *['prototype']() {} }", "class Foo { *prototype() { }}");
// test("class Foo { get ['prototype']() {} }", "class Foo { get prototype() { }}");
// test("class Foo { set ['prototype'](x) {} }", "class Foo { set prototype(x) { }}");
// test("class Foo { async ['prototype']() {} }", "class Foo { async prototype() { }}");
// test("class Foo { static ['prototype'] = 0 }", "class Foo { static ['prototype'] = 0;}");
// test("class Foo { static ['prototype']() {} }", "class Foo { static ['prototype']() { }}");
// test("class Foo { static *['prototype']() {} }", "class Foo { static *['prototype']() { }}");
// test(
// "class Foo { static get ['prototype']() {} }",
// "class Foo { static get ['prototype']() { }}",
// );
// test(
// "class Foo { static set ['prototype'](x) {} }",
// "class Foo { static set ['prototype'](x) { }}",
// );
// test(
// "class Foo { static async ['prototype']() {} }",
// "class Foo { static async ['prototype']() { }}",
// );
// test(
// "class Foo { constructor() {} ['constructor']() {} }",
// "class Foo { constructor() { } ['constructor']() { }}",
// );
// test(
// "class Foo { static constructor() {} static ['constructor']() {} }",
// "class Foo { static constructor() { } static constructor() { }}",
// );
test("class Foo { ['constructor'] = 0 }", "class Foo { ['constructor'] = 0;}");
test("class Foo { ['constructor']() {} }", "class Foo { ['constructor']() { }}");
test("class Foo { *['constructor']() {} }", "class Foo { *['constructor']() { }}");
test("class Foo { get ['constructor']() {} }", "class Foo { get ['constructor']() { }}");
test("class Foo { set ['constructor'](x) {} }", "class Foo { set ['constructor'](x) { }}");
test("class Foo { async ['constructor']() {} }", "class Foo { async ['constructor']() { }}");
test("class Foo { static ['constructor'] = 0 }", "class Foo { static ['constructor'] = 0;}");
test("class Foo { static ['constructor']() {} }", "class Foo { static constructor() { }}");
test("class Foo { static *['constructor']() {} }", "class Foo { static *constructor() { }}");
test(
"class Foo { static get ['constructor']() {} }",
"class Foo { static get constructor() { }}",
);
test(
"class Foo { static set ['constructor'](x) {} }",
"class Foo { static set constructor(x) { }}",
);
test(
"class Foo { static async ['constructor']() {} }",
"class Foo { static async constructor() { }}",
);
test("class Foo { ['prototype'] = 0 }", "class Foo { prototype = 0;}");
test("class Foo { ['prototype']() {} }", "class Foo { prototype() { }}");
test("class Foo { *['prototype']() {} }", "class Foo { *prototype() { }}");
test("class Foo { get ['prototype']() {} }", "class Foo { get prototype() { }}");
test("class Foo { set ['prototype'](x) {} }", "class Foo { set prototype(x) { }}");
test("class Foo { async ['prototype']() {} }", "class Foo { async prototype() { }}");
test("class Foo { static ['prototype'] = 0 }", "class Foo { static ['prototype'] = 0;}");
test("class Foo { static ['prototype']() {} }", "class Foo { static ['prototype']() { }}");
test("class Foo { static *['prototype']() {} }", "class Foo { static *['prototype']() { }}");
test(
"class Foo { static get ['prototype']() {} }",
"class Foo { static get ['prototype']() { }}",
);
test(
"class Foo { static set ['prototype'](x) {} }",
"class Foo { static set ['prototype'](x) { }}",
);
test(
"class Foo { static async ['prototype']() {} }",
"class Foo { static async ['prototype']() { }}",
);
test(
"class Foo { constructor() {} ['constructor']() {} }",
"class Foo { constructor() { } ['constructor']() { }}",
);
test(
"class Foo { static constructor() {} static ['constructor']() {} }",
"class Foo { static constructor() { } static constructor() { }}",
);
test("class x { '0' = y }", "class x { 0 = y;}");
test("class x { '123' = y }", "class x { 123 = y;}");
test("class x { ['-123'] = y }", "class x { '-123' = y;}");
Expand Down
Loading