feat(transformer/class-properties): transform super assignment expressions that are inside static prop initializer

[Dunqing]

No description provided.

[Dunqing]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• feat(transformer/class-properties): transform super assignment expressions that are inside static prop initializer #7959 👈 (View in Graphite)
• refactor(transformer/class-properties): make transform_instance_assignment_expression more generic #7956
• docs(transformer/class-properties): remove oudated todo for assignment expression #7955
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codspeed-hq]

CodSpeed Performance Report

Merging #7959 will not alter performance

_{Comparing 12-17-feat_transformer_class-properties_transform_super_assignment_expressions_that_are_inside_static_prop_initializer (a949281) with 12-17-refactor_transformer_class-properties_make_transform_instance_assignment_expression_more_generic (8a4be03)}

Summary

✅ 29 untouched benchmarks

[overlookmotel]

This is shorter, but I'm not really a fan of impl From<IdentifierName> for StringLiteral.

Reason is: You're dealing with an owned type on the stack. Usually, it's better to allocate nodes into the arena as quickly as possible, and then only hold smaller types like:

• Box<T> (8 bytes)
• References &T or &mut T (8 bytes)
• Enum types e.g. Expression, Statement (16 bytes)
• Atom (16 bytes)

Generally, if you find yourself having to use ctx.ast.alloc, you're not in an optimal situation, because it means you have a large type on the stack.

In the original version, you only had to clone the Atom (which is essentially a Copy), whereas in this new version you have to clone the whole IdentifierName to be able to pass it to from.

In this case, it's not a big deal, because IdentifierName is only 24 bytes, but in general in my opinion it's not the best pattern.

If you wanted to, you could implement this conversion instead as a method on AstBuilder - AstBuilder:: expression_string_literal_from_identifier_name. But personally I think that'd be overkill unless we need to do that conversion in many places.

[overlookmotel]

I think this is the cause of the panic in conformance. Problem is that transform_assignment_expression sometimes mutates expr so it's not an AssignmentExpression any more. So then transform_super_assignment_expression enters unreachable!().

So we need to check that it's still an AssignmentExpression:

Suggested change

	self.class_properties.transform_super_assignment_expression(expr, self.ctx);
	if matches!(expr, Expression::AssignmentExpression(_)) {
	self.class_properties.transform_super_assignment_expression(expr, self.ctx);
	}

Having to do this check again is inefficient, but don't worry - this visitor will be removed anyway in my forthcoming refactoring, so we can change it then.