Skip to content
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.

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

externs for getters and setters #1

Open
kennylerma opened this issue May 19, 2018 · 8 comments
Open

externs for getters and setters #1

kennylerma opened this issue May 19, 2018 · 8 comments
Assignees

Comments

@kennylerma
Copy link

kennylerma commented May 19, 2018

Currently, the below javascript will produce a public variable in the generated ActionScript using externc. How do I notate the javascript to become a getter or setter?

/**
 * @type {number}
 */
createjs.MovieClip.prototype.currentFrame;
@aharui aharui self-assigned this May 22, 2018
@aharui
Copy link
Contributor

aharui commented May 22, 2018

It looks like the compiler only generates getter/setters if the variable is also implemented in an interface. That's because interfaces can only have getter/setter functions and not vars.

In theory, code calling into the external class shouldn't care whether it is a var or getter/setter. What is the use case where it matters?

@kennylerma
Copy link
Author

kennylerma commented May 22, 2018

In my particular use case, I need to override a getter method. Additionally, I want to match the external library as closely as possible to catch compile time errors like attempting to set a read only variable.

I hoping I can contribute a bit to this repo by completing out the CreateJS library and possible more, but I'll need ability to create the getters and setters. Something simple might just be adding the @readonly to generate a getter.

/**
 * @type {number}
 * @readonly
 */
createjs.MovieClip.prototype.currentFrame;

@aharui
Copy link
Contributor

aharui commented May 22, 2018

It would be great to see CreateJS support get more attention.

I was guessing that you are trying to subclass an extern class. Royale's support for that is definitely in need of improvement. We could probably come up with some way to specify that the API is backed by getter/setter, but first, I would like to know what you would want the output .JS to be. Royale's current code for overriding getter/setters relies on the base class having been generated by Royale so that the actual function has a known prefix. IOW

public function get foo() {}

is output (roughly) as:

BaseClass.prototype.get__foo() {}
Object.defineProperty(BaseClass.prototype, "foo", get_foo);

Then the override:

override public function get foo() { return super.foo; }

is output (roughly) as:

SubClass.prototype.get__foo() { return BaseClass.prototype.get_foo(); }
Object.defineProperty(SubClass.prototype, "foo", get_foo);

This was found to be significantly faster than doing overrides in the Object.defineProperties structures, but relies on the compiler knowing that the base class has a "get__" or "set__" method.

We will need to know what CreateJS uses as the override pattern. We also need to know if CreateJS has a policy or pattern for overriding only one of the getters or setters in the base class. If the base class has both a getter and setter, but you only override the getter in your base class, if we call Object.defineProperty with only the getter, you end up with a read-only property. The Object.defineProperty either has to pick up the base class's setter and use it in Object.defineProperty or generate a small setter that calls the base class setter.

So if you can post some snippets of how CreateJS manages getter/setters and overrides we can see what it will take to get the compiler to generate that kind of code.

@kennylerma
Copy link
Author

Ah, I guess I should have expected it to be more complicated. :)

This is how CreateJS sets up its getters/setters

Object.defineProperties(p, {
	totalFrames: { get: p._getDuration }
});

So, Royale would need to generate new methods to assign the getters/setters to and keep the Superclass methods in-tact.

p._customTotalFrames = function() {
	console.log("Superclass value: " + this._getDuration());
	return 200; // return custom Subclass value.
}

Object.defineProperties(p, {
	totalFrames: { get: p._customTotalFrames }
});

@aharui
Copy link
Contributor

aharui commented May 23, 2018

Interesting. I'm not sure I see a pattern. Do you see a pattern? The compiler needs to know the pattern (or have enough directives) in order to know what to output. What manual process do you go through when subclassing a CreateJS class and overriding getter/setters so you know you won't bash the access to the superclass methods? Having to know what they are and picking a unique name is not well-suited for compiler code-generation.

@kennylerma
Copy link
Author

Yeah, CreateJS has it's own utility class for overriding methods which of course wouldn't be used here.

However, I was thinking you could using something like this to call the super class method.

Object.getOwnPropertyDescriptor(obj, 'MyField').get;  // returns the superclass get function

/* setter */
Object.getOwnPropertyDescriptor(obj, 'MyField').set;  // returns the superclass set function

// so super.MyField would return the original method.
super.MyField -> Object.getOwnPropertyDescriptor(obj, 'MyField').get;

Could that work?

@aharui
Copy link
Contributor

aharui commented May 23, 2018

The compiler can be told to output different patterns. So if there is a pattern to how you use a CreateJS utility class, a custom "emitter" for CreateJS could do so.

I could use the pattern you describe above, but I think we used to output something like that and found it was very slow. But maybe that's as good as it gets. IIRC, the actual pattern has to be a bit more complex in the case where the superclass doesn't have the API that is being overridden, the superclass's superclass does. I think the pattern has to see if the Object.getOwnPropertyDescriptor return null/undefined and then loop through the ancestor classes.

A second problem is knowing what name to give to your override. Currently we generate a get__foo and set__foo (yes, two underscores) but we don't have any guarantee that someone else might have already used that name in one of the base classes.

If you can manually change some of the APIs to use a proposed pattern of Object.getOwnPropertyDescriptor so you know it works, then we know if we teach the compiler to output the same that it will be worth the effort.

And bonus if you or some other volunteer wants to learn enough about how the compiler code generator works to make these changes.

@kennylerma
Copy link
Author

Thanks @aharui, I'm going to create a working example and see what works best. I've been trying to learn about compilers lately and I'm certainly interested in the Royale Compiler. I'll have to try building it myself and see how far I get.

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

No branches or pull requests

2 participants