You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Class properties transform is now complete except for some edge cases.
Conformance now runs without panic, and monitor-oxc seems to show only remaining issues relate to the transforms we've not done yet.
We have 2 more class transforms to implement:
Private methods
#prop in object
We should NOT enable class properties transform (#7750) until these other 2 transforms are complete, because they need to work in concert with each other.
How the 3 transforms work together
It looks to me like private methods transform and #prop in object transform could run alone. But class properties transform requires the other 2 transforms to work correctly.
Otherwise you have cases like this where properties transform is enabled, and private methods transform is not:
We now have _C.#privateMethodoutside the class, which is a syntax error.
We could fix that and only transform methods which need to be transformed. But it'd be very slow because you don't know if a method needs to be transformed or not until you find a reference to it in a static prop, and then you'd need to re-visit the whole class again to find any other references to this private method and transform them too.
So I suggest that we enable private methods + #prop in object transforms automatically if class properties transform is enabled.
Testing
To make Babel's tests for the 2 new transforms work, change the update_fixtures.mjs script to add class-properties to plugins if private-methods or private-property-in-object plugin is there.
Implementation: #prop in object
This is much easier than private methods, but it depends on private methods, because #method in object is also valid. So in one way it makes more sense to do private methods first. BUT, on other hand, doing this transform first would be a good way to explore all the parts of the transform, and get a feel for it.
I think the existing structures PrivateProp and ClassBindings contain all the info needed for this transform, without any alteration.
Implementation: Private methods
I think best to build this inside the class properties transform. It can use the same data structures.
#8042 adds an is_method property to PrivateProp, and records private methods in ClassDetails::private_props hash map, same as private properties:
object.#method needs to be transformed anywhere where ClassesStack::find_private_prop is called. Currently there's always an if is_method { return; } line after that call. e.g.:
That's where transform of this.#method would slot in.
"Brand" temp var
If class has any instance private methods (not static), then an extra temp var is created var _Class_brand = new WeakSet();. There is 1 "brand" temp var per class, not 1 per method. Then transpiled #object.method uses this var.
I suggest:
Record the binding in ClassBindings as a brand: Option<BoundIdentifier<'a>> property, along with name and temp.
To match Babel's output (order of where var _Class_brand = ...; is inserted), will need to insert it in same loop as where temp vars for private properties are inserted. When you find the first PrivateProp with is_method == true, insert var _Class_brand then.
Transforming methods themselves
Don't do anything to private methods in entry phase of transform. Do it all in exit phase (ClassProperties::transform_class_elements), same as for static properties + static blocks. That will allow other transforms to run on the method's body before it's moved to outside of the class.
The body of the method needs to be transformed as well to replace references to class name with temp var, and transform super:
The _method function can just be moved, and its parent scope ID updated. If the context outside the class is not strict mode, then all scopes within the method need to have ScopeFlags::StrictMode flag removed.
Luckily, this is all extremely similar to the transform that happens on static property initializers, using StaticVisitor.
The differences are:
this does not need to be transformed.
Scopes within method body (e.g. nested blocks or functions) do not need their parent scope updated (because they remain within the same function).
super.prop is transformed slightly differently if it's an instance method.
Could either:
Copy-and-paste StaticVisitor and amend it. or
Alter StaticVisitor to also cover private methods.
Eventually, we should move all this into the main visitor, to remove these extra visits. But I strongly suggest not to do that initially. It will be challenging to track all the required state. Will be easier to do once tests are passing.
Summary
There's quite a lot to this transform, but on positive side, it mostly follows same patterns as we already have in class properties transform.
And if any consolation, at least no need to deal with computed keys for private methods!
The text was updated successfully, but these errors were encountered:
Class properties transform is now complete except for some edge cases.
Conformance now runs without panic, and monitor-oxc seems to show only remaining issues relate to the transforms we've not done yet.
We have 2 more class transforms to implement:
#prop in object
We should NOT enable class properties transform (#7750) until these other 2 transforms are complete, because they need to work in concert with each other.
How the 3 transforms work together
It looks to me like private methods transform and
#prop in object
transform could run alone. But class properties transform requires the other 2 transforms to work correctly.Otherwise you have cases like this where properties transform is enabled, and private methods transform is not:
We now have
_C.#privateMethod
outside the class, which is a syntax error.We could fix that and only transform methods which need to be transformed. But it'd be very slow because you don't know if a method needs to be transformed or not until you find a reference to it in a static prop, and then you'd need to re-visit the whole class again to find any other references to this private method and transform them too.
So I suggest that we enable private methods +
#prop in object
transforms automatically if class properties transform is enabled.Testing
To make Babel's tests for the 2 new transforms work, change the
update_fixtures.mjs
script to addclass-properties
toplugins
ifprivate-methods
orprivate-property-in-object
plugin is there.Implementation:
#prop in object
This is much easier than private methods, but it depends on private methods, because
#method in object
is also valid. So in one way it makes more sense to do private methods first. BUT, on other hand, doing this transform first would be a good way to explore all the parts of the transform, and get a feel for it.I think the existing structures
PrivateProp
andClassBindings
contain all the info needed for this transform, without any alteration.Implementation: Private methods
I think best to build this inside the class properties transform. It can use the same data structures.
#8042 adds an
is_method
property toPrivateProp
, and records private methods inClassDetails::private_props
hash map, same as private properties:oxc/crates/oxc_transformer/src/es2022/class_properties/class_details.rs
Lines 40 to 45 in c46793e
Transforming
this.#method
object.#method
needs to be transformed anywhere whereClassesStack::find_private_prop
is called. Currently there's always anif is_method { return; }
line after that call. e.g.:oxc/crates/oxc_transformer/src/es2022/class_properties/private_field.rs
Lines 54 to 65 in c46793e
That's where transform of
this.#method
would slot in."Brand" temp var
If class has any instance private methods (not static), then an extra temp var is created
var _Class_brand = new WeakSet();
. There is 1 "brand" temp var per class, not 1 per method. Then transpiled#object.method
uses this var.I suggest:
ClassBindings
as abrand: Option<BoundIdentifier<'a>>
property, along withname
andtemp
.oxc/crates/oxc_transformer/src/es2022/class_properties/class_bindings.rs
Lines 41 to 54 in c46793e
ClassProperties::transform_class_body_on_entry
)._classPrivateMethodInitSpec(this, _Class_brand);
into class constructor in entry phase, by adding it toinstance_inits
:oxc/crates/oxc_transformer/src/es2022/class_properties/class.rs
Lines 252 to 274 in c46793e
var
statement for it in exit phase of the transform. This is in 2 places (transform_class_declaration_on_exit_impl
andtransform_class_expression_on_exit_impl
).var _Class_brand = ...;
is inserted), will need to insert it in same loop as where temp vars for private properties are inserted. When you find the firstPrivateProp
withis_method == true
, insertvar _Class_brand
then.Transforming methods themselves
Don't do anything to private methods in entry phase of transform. Do it all in exit phase (
ClassProperties::transform_class_elements
), same as for static properties + static blocks. That will allow other transforms to run on the method's body before it's moved to outside of the class.The body of the method needs to be transformed as well to replace references to class name with temp var, and transform
super
:The
_method
function can just be moved, and its parent scope ID updated. If the context outside the class is not strict mode, then all scopes within the method need to haveScopeFlags::StrictMode
flag removed.Luckily, this is all extremely similar to the transform that happens on static property initializers, using
StaticVisitor
.The differences are:
this
does not need to be transformed.super.prop
is transformed slightly differently if it's an instance method.Could either:
StaticVisitor
and amend it. orStaticVisitor
to also cover private methods.Eventually, we should move all this into the main visitor, to remove these extra visits. But I strongly suggest not to do that initially. It will be challenging to track all the required state. Will be easier to do once tests are passing.
Summary
There's quite a lot to this transform, but on positive side, it mostly follows same patterns as we already have in class properties transform.
And if any consolation, at least no need to deal with computed keys for private methods!
The text was updated successfully, but these errors were encountered: