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

Adding support for ExternalInterface #325

Closed
wants to merge 0 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 25, 2018

Adding in a class to allow Royale applications to communicate with other JavaScript functions on the hosting webpage. Uses the same API as the Flash ExternalInterface (and the SWF build uses this implementation). JS version just calls the function given directly; callbacks are added to a special hidden element so that the other JavaScript code can call the ActionScript function using the same form as it would for the Flash Player.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

Interesting idea. I was expecting it to be in the MXRoyale SWC in mx/core.

Putting it in Core.swc implies it is a pattern we want people to use in new projects, not just projects migrating from Flex. While it would work, I wonder if we should be promoting some other, more tightly integrated pattern like typedefs/externs so you can maintain strong-typing when calling external code.

ExternalInterface loses the type-safety. It returns * and you have to cast/coerce the results. If you instead just create the AS API for your JS functions and add @externs to the ASDoc, the compiler should transpile the AS to JS but treat it as a typedef so it won't actually put the transpiled code in the output.

What do others think?

@piotrzarzycki21
Copy link
Member

+1 for having that as part of the externs.

@ghost
Copy link
Author

ghost commented Oct 25, 2018

The idea of defining the API in ActionScript and keeping the type information sounds good.
I couldn't see any examples of using @externs in ActionScript anywhere, the only thing I could see is a file-level directive within some JavaScript files. I've been trying to get this to work with an AS file but if it's at file scope, it just gets ignored, and if I add it to a function then I get a closure compiler internal error. I'm assuming that this is actually the Royale transpiler that should be removing the function definition when transpiling, so presumably it shouldn't actually create a .js file for it?

I'm slightly unclear how to get the external JavaScript code to call a method within the Royale application still; we would still need to register a callback somehow I guess, or else we'd just have to call the qualified function but without any 'this' parameter. To keep it working with a method closure I believe the "registering a callback" approach is required so we need to define a function somewhere for this - perhaps this then needs to be added to the AS externs code (but without the @extern tag..)

Presumably all of this approach works just for when we compile our apps into JavaScript/HTML, and the application becomes responsible for the compile-switch between JS and SWF and then continues to use flash.external.ExternalInterface in a COMPILE::SWF block...

thanks

@carlosrovira
Copy link
Contributor

On strong point to go with Royale instead with plan JS is string typing, so I think we should have ExternalInterface for MX and maybe other implementation for new code. For example, the principal use I did was through the use RemoteObjects and BlazeDS does and is part of the Small Messages that we still need to bring to MX implementation. Thanks for working on this, is other missed and important point.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

Here is a "source externs" I put together for a Cordova plugin. https://github.com/apache/royale-tourjs/blob/develop/cordova/BatchScanner/barcodeScanner/src/main/flex/cordova/plugins/barcodeScanner.as

It might be that we need to do more work to support "global" JavaScript functions.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

Every public class and method/property is currently available to JavaScript. So if you have org.apache.royale.html.Alert.show() in AS, you can also just call it from JS. I don't think you need to register a closure as a callback. I think JS will just see it as any other JS call and set up the 'this' pointer based on any instances referenced from JS. Needs testing though.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

In theory, when using ExternalInterface in JS, whatever method you pass into the addCallback method will be converted to a closure by the compiler. The compiler is supposed to generate org.apache.royale.utils.Language.closure() calls for parameters of type Function.

@ghost
Copy link
Author

ghost commented Oct 25, 2018

Great, thanks for those ... will take a look and update here when I've got this working...

@ghost
Copy link
Author

ghost commented Oct 25, 2018

One quick comment having looked at the barcode scanner example: the "@externs" tag is on the class comment. It looks like (from PackageHeaderEmitter.java) the tag is only checked for classes and interfaces; I'm looking at a top-level package with just a function inside it. Might see if I can fool it by also defining an empty class... otherwise I'm looking at the compiler code to see if it could also check package-level comments/markup for this.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

Yeah, that's what I meant by "we need to do more work to support "global" JavaScript functions".

Search the compiler code base for @externs. It should show the relevant files. I think you can adjust where we look for @externs in PackageHeaderEmitter.java

I'm done for today so won't respond for a while.

@ghost
Copy link
Author

ghost commented Oct 25, 2018

Okay, so with a slight change in PackageHeaderEmitter.java (to check for the @externs tag when looking at a function definition) and a change also in MethodEmitter.java (to prevent us emitting a method that's got package scope and has the '@externs' tag), then we end up with code that transpiles and produces sensible JS output, and even works in the browser, for debug builds.
As soon as we then try the release output, there's a closure compiler error, as the code that's calling this function has a 'goog.require' call for it, but we don't have anything providing this now...

There's another limitation with this approach, which is that we can't have multiple public top-level functions defined within an ActionScript file. There's some checks going on in ASFileScopeRequestResult.checkExternallyVisibleDefinitions() which are ensuring we only have one public definition, and this needs to match the filename. So having an API defined in this way is a bit of a non-started.

So I'm thinking that a better approach might be to define a class for our external-JavaScript API and put the functions into this. This way, we can tag the class as @externs and shouldn't need to amend the compiler at all! Probably a little more work to do in terms of the JS integration but I'll take a look and see what I find...

@ghost
Copy link
Author

ghost commented Oct 25, 2018

Hmmm. Tagging a class as @externs has its own issues. Firstly the transpiler still generates the functionality into the js-debug folder, it just puts the @externs tag at the top of the file. So this means we have empty functions causing confusion when trying to debug (and I had a 'trace' statement in my function which really caused some issues due to the results goog.require call...). Secondly however, this is causing a problem in the release builds, as there isn't a "goog.provide" call, so the class that's importing this then faces an issue.

This looks like it's the same with the barcode scanner that you pointed me at; I've copied that into my own simple project. The js-debug version compiles - and leaves me with an empty function for the 'scan' method - but when I ask for a release build, I then get this closure compiler error. I noticed that the build.xml file for this component has got "-debug" hardcoded into it so perhaps this hasn't been tried before (or it was 2 years ago..?)

So I have a kind of solution which is a bit simpler than having to do the whole ExternalInterface class definition, although it borrows a bit from this in terms of the callback..

If we define our API in a normal class (i.e. forgetting about @externs) and add the glue directly in here to call our JavaScript functions:
public static function addNumbers(a:int,b:int):int { return window["addNumbers"](a,b); }
then we can call out using this e.g. if the class was called "extAPI" then we call
val result : int = extAPI.addNumbers(3, 4);

To get a callback working, we need to define an extra function in our extAPI class:
public static function registerCallback(fncName : String, methodClosure : Function) : void { extAPI[fncName] = methodClosure; }
and this we then set up from our ActionScript:
extAPI.registerCallback("myFunction", runCallback);
and our external JavaScript can call it simply with:
extAPI.myFunction("999");

So that all works and perhaps is an alternative to needing the ExternalInterface class. Although I still think it might be worth having this in the RoyaleMX package perhaps, to aid in migration..!

Let me know what you think.
Thanks

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

Having ExternalInterface as you originally proposed it in the mx.core package and in the MXRoyale SWC is definitely worth doing.

Regarding the problems with @externs in a file, it seems like we just need to do a little more work in the compiler to 1) have the output go in an externs folder or wherever it is we currently place the externs from the typedefs swcs, and make sure the goog.require does not go in the files calling the extern.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

It is true that there is a limitation on multiple top-level definitions in AS. IIRC, you can find advise on the internet about the risks of having top-level JS APIs (collisions and 'stomping') and the general advice is to always have your JS code use some sort of "package".

It may be that there isn't one recommended practice for calling code in the hosting page. We should fix the compiler so you can use typedefs. Folks will be able to use ExternalInterface if we move it to the mx.core package and put it in MXRoyale, and they could follow your example if they want as well.

@ghost
Copy link
Author

ghost commented Oct 25, 2018

Thanks - so I will update this to define it as mx.core.ExternalInterface; I might try looking into the @externs thing although there are a few more pressing issues to sort out still!

Yes I agree it's better practise to have your JavaScript functionality encapsulated into 'packages'. And it's feasible to create this sort of wrapper between ActionScript and JavaScript without too much hassle.. probably a wiki entry on this might be useful..!

I guess in the meantime, this pull request can be closed/ignored..
Thanks

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

I think you can just update this PR with the code in mx.core and changing a MXRoyaleClasses instead of CoreClasses. Whatever is easier for you.

@aharui
Copy link
Contributor

aharui commented Oct 25, 2018

Also, a chapter in our docs would be better than a wiki article. IMO the wiki is for folks making changes to the repos and the royale-docs repo is for user-facing documentation. Feel free to submit a PR for royale-docs with your example.

@ghost ghost closed this Oct 29, 2018
@ghost ghost force-pushed the ajwfrost/externalinterface branch from 6a17fce to 18912bd Compare October 29, 2018 11:29
@ajwfrost
Copy link

Accidentally closed this, will open another PR for the new implementation for ExternalInterface in the MXRoyale project. FYI there was already a skeleton implemention in mx.external.ExternalInterface, not sure how I missed spotting that!!

This pull request was closed.
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.

None yet

4 participants