Skip to content

Support transpiling class methods as named functions#1548

Merged
luis-j-soares merged 9 commits intomasterfrom
feature/class-method-hoisting
Aug 11, 2025
Merged

Support transpiling class methods as named functions#1548
luis-j-soares merged 9 commits intomasterfrom
feature/class-method-hoisting

Conversation

@luis-j-soares
Copy link
Collaborator

@luis-j-soares luis-j-soares commented Aug 5, 2025

Fixes #1544

This PR changes the transpiled output of classes from this:

function __MyKlass_builder()
    instance = {}
    instance.new = sub()
    end sub
    instance.abc = function() as integer
    end function
    instance.def = function() as object
    end function
    return instance
end function
function MyKlass()
    instance = __MyKlass_builder()
    instance.new()
    return instance
end function

To this:

sub __MyKlass_new()
end sub
function __MyKlass_abc() as integer
end function
function __MyKlass_def() as object
end function
function __MyKlass_builder()
    instance = {}
    instance.new = __MyKlass_new
    instance.abc = __MyKlass_abc
    instance.def = __MyKlass_def
    return instance
end function
function MyKlass()
    instance = __MyKlass_builder()
    instance.new()
    return instance
end function

Notes:

  • On a call with Bronley and Chris, it was decided that the order of transpilation would be:
    1. Methods (__[ClassName]_[methodName])
    2. Builder (__[ClassName]_builder)
    3. Factory ([ClassName])
  • Added a new argument to MethodStatement::transpile to allow for a custom name to be passed in. Which in this case, this is used to pass the method name concatenated to the class prefix.
  • The builder function was cloning the body to add a constructor, which was implemented in Fix class transpile issue with child class constructor not inherriting parent params #1390. I moved that to a separate function and call it on transpile, as we need to reuse that body for both the methods and the builder functions.

Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To keep the diff a little smaller, it might be good to keep any existing functions in their current locations rather than moving them around. Good example is getTranspiledBuilder, it seems like it moved? This'll make the merge into v1 a lot easier as well.

);
}

private getBuilderName(className: string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably rename this param to transpiledClassName since that's what you're expecting. Same with the getMethodIdentifier function.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed for consistency. Thanks!

@luis-j-soares
Copy link
Collaborator Author

To keep the diff a little smaller, it might be good to keep any existing functions in their current locations rather than moving them around. Good example is getTranspiledBuilder, it seems like it moved? This'll make the merge into v1 a lot easier as well.

Good point. I didn't exactly move getTranspiledBuilder, but I added a few new methods above it and the diff viewer doesn't seem to take that well. 😅

I've now moved those new methods below the builder, and the diff seems easier to parse.

Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just thought of an edge case. What happens if you have a method called builder?

class Person
    sub builder()
    end sub
end class

I think the current implementation will result in two __Person_builder functions. We probably need a way to differentiate (without back-compat breaks) the actual builder, and methods with the name builder. So maybe another underscore for methods? or "method" or something in the name. Thoughts?

@luis-j-soares
Copy link
Collaborator Author

Just thought of an edge case. What happens if you have a method called builder?

class Person
    sub builder()
    end sub
end class

I think the current implementation will result in two __Person_builder functions. We probably need a way to differentiate (without back-compat breaks) the actual builder, and methods with the name builder. So maybe another underscore for methods? or "method" or something in the name. Thoughts?

As discussed offline, renamed the hoisted methods to __${className}_method_${methodName}.

@luis-j-soares luis-j-soares merged commit c0be575 into master Aug 11, 2025
7 checks passed
@luis-j-soares luis-j-soares deleted the feature/class-method-hoisting branch August 11, 2025 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support transpiling class methods as named functions

2 participants