After looking at some of the code, I remembered that the DependencyGraph generated by the compiler is not useful for JS output as it currently is. The DependencyGraph is tuned for SWF code where class and instance properties have types and those types are verified by the runtime. IOW, if UIComponent has a "public var foo:SystemManager", then in SWF code, UIComponent is dependent on SystemManager, but in JS it isn't because the JS output is just "var foo". The SystemManager class doesn't even have to be in the JS output (until someone actually instantiates it or references a static API). So any ordering that is useful for SWF is probably not the same order for JS. It might be true that SWF order is valid for JS, but I think I found that isn't quite true once, although I might be mis-remembering that.
I don't think there is a way to know a class is about to be used without more sophisticated (and thus slower) code, so we've so far not gone down that road. I believe that the compiler will catch the circularities that are truly illegal in SWF (class Foo extends Bar and class Bar extends Foo, and similar things where static const Foo.baz = Bar.baz and static const Bar.baz = Foo.baz. And more complex variants where the Flash runtime really could not do the right thing. There may be more illegal situations in JS where static APIs in different classes in JS depend on each other, but many are legal and we just need to remove circulars differently by giving priority to static usage. I think we may not need to catch truly illegal static initializer scenarios in JS at compile/emitter time and can rely on errors generated by GoogDepsWriter and Google Closure Compiler to catch those circularities (because the provides/requires will be circular or GoogDepsWriter can see they are circular). So, IMO, the next set of improvements are really just around detecting more static dependencies at Emitter time and handing that information to GoogDepsWriter. I don't think we need to change the actual JS we output. I'm certain that we can detect that we're emitting static initializer code, but I don't think the compiler currently detects it in a way that the rest of the emitter code can track dependency usage differently. My 2 cents, -Alex On 7/20/18, 8:10 AM, "Frost, Andrew" <[email protected]> wrote: Yes good point: so to protect ourselves it might be that we need to try to mimic what the avmplus does and just call a 'class initialiser' method to set up all the static properties, when the class is first referenced. Although I'm not sure whether we can detect from JavaScript when the class is first referenced..?! In terms of trying to make the dependency emitter a little more clever: aren't we always going to face problems if there really are two classes that have a reference to each other? So a real circular reference that can't be resolved? Maybe this isn't so common in real life... I guess if it doesn't yet know that it's emitting a static initializer then yes there's a more basic issue to address! thanks Andrew -----Original Message----- From: Alex Harui [mailto:[email protected]] Sent: 20 July 2018 15:55 To: [email protected] Subject: [EXTERNAL] Re: Query on Royale dependency generation Personally, I'm not convinced the problem is truly different in SWF vs JS, other than that in JS, the google closure compiler and goog.provides/requires/addDependency don't really let you specify the difference between static and non-static initializer dependencies. I think you may have seen that when you played with the "verifyAll" setting. The main difference, AIUI, is that the Flash runtime only initializes the statics on a class when first used instead of when loaded. JS initializes all statics when loaded. But I think that the order in many cases still matters in the SWF. I see code in the SWF output section of the compiler that figures out the order of classes to output into the SWF. There is a DependencyGraph that is managed by the compiler. It is possible that the problem is simply a matter of translating that SWF order into the goog.provides/requires/addDependency. I think that at Emitter time, we can know that we are initializing a static initializer and can record that information in the output for use in GoogDepsWriter. I worry that the workaround in #2 may not always work. The workaround changes when the initializer runs, but I wonder if there will be scenarios where the initializers are still going to be needed "early". Whether we do #2 or #3, the Emitter logic still needs to detect that it is outputting a static initializer. So it isn't clear that it is worth the overhead of outputting different code vs just leaving more data for GoogDepsWriter. My 2 cents, -Alex On 7/20/18, 6:49 AM, "Frost, Andrew" <[email protected]> wrote: Hi guys Thanks for the details. Harbs, I see what you mean now, and yes I can work around it in the source code by changing how the variable is initialised. I guess circular dependencies are a little tricky from a JavaScript perspective! It's easier in ActionScript due to the way the player sets things up (although when we tried the "verifyAll" setting in avmplus, we ran into similar problems with some SWCs...) In terms of the options, I guess my preference would be '2', where the static initialisers are output using the pattern that Harbs suggests. For now I can switch to that pattern manually, but I can add that idea to the list of things to investigate when we have more time.. thanks Andrew -----Original Message----- From: Alex Harui [mailto:[email protected]] Sent: 19 July 2018 18:04 To: [email protected] Subject: [EXTERNAL] Re: Query on Royale dependency generation For sure, there are probably still bugs in the remove-circulars calculation. But, before debugging these problems, make sure you remove the entire bin/js-debug folder. The JS files in there are modified by the dependency management code and could get stale. The fundamental problem is that goog.provides, goog.requires, and goog.addDependency result in <script> tags being added at runtime (in debug mode) to bring in the JS files needed by the classes. Similarly, the Google Closure Compiler orders the classes in the minified output so that initializers are ready to be used when needed. The Closure Compiler used to complain when it detected a circularity (when a goog.provides("A") has goog.requires("B") but goog.provides("B") has goog.requires("A"). Maybe they turned that off in a recent version? Because I would have expected the Closure Compiler to complain when Andrew set remove-circulars=false. ActionScript disallows certain kinds of circular dependencies as well, but many are allowed in AS and are actually legal in JS that the Closure Compiler complains about. So the whole goal of the GoogDepsWriter is to detect the legal circularities and rewrite the output JS files to make Closure Compiler happy. So we do that by putting lots of goog.requires in the main application in 'the right order'. The GoogDepsWriter tries to detect certain kinds of static initializers and overrides the ordering. I think the pattern Andrew is using isn't currently detected. Harbs' suggestion of having the compiler output static initializers differently might work, but also might just move the problem. Sometimes, a simple app exposes more of these issues that a complex one because the complex one often has enough code that also uses one of the classes involved that it modifies the order of goog.requires in the big list in the main jS file. So, IMO, the choices are: 1) just have folks workaround the issue using the pattern Harbs showed 2) output static initializers differently 3) do better detection of static initializers in GoogDepsWriter Thoughts? -Alex On 7/19/18, 7:11 AM, "Harbs" <[email protected]> wrote: Here’s the short version: All the files are loaded in order of the dependency declarations. Calculating the correct order is very difficult and sometimes impossible. The way the circular dependencies are resolved is by simply declaring them all in the main class. Declaring them in subclasses can cause goog to try and load files before they are a actually loaded. The exact order ends up being a bit of a crapshoot. Statics are evaluated when the file is first loaded, so initializing anything other than native (or externally loaded) objects can cause an error. My proposed solution is to extract all non-native costs and initialized vars into a separate file and load that file last in the dependency list. That ensures all the classes are already loaded. Unfortunately no-one has worked on resolving this problem yet. Earlier versions of the compiler tried to be smarter about order, but that turned out to not always work… Alex could give you the longer version… ;-) What I do for cases where static custom classes are needed is something like this: private static var _myFoo:Foo; public static function get FOO():Foo{ if(!_myFoo){ _myFoo = new Foo(); } return _myFoo; } There’s slightly more overhead than a const, but at least it works… Harbs > On Jul 19, 2018, at 4:39 PM, Frost, Andrew <[email protected]> wrote: > > Hi > >> If it’s in the big list, it should be initialized correctly. > .. but depending on the order of the big list, things will be initialised in different orders... > >> Are you initializing a BinaryData as a static const (or preinitialized var)? > Yes >> i.e. public static const DATA:BinaryData = new BinaryData() is a no-no. > So - why is this a no-no? I'm trying to convert an ActionScript class that Adobe wrote a long time ago, which has the equivalent using ByteArray. Are you saying that it's not possible to do certain things with ActionScript in Royale that are fine when you're targeting Flash? > I know it's not a great practice to have this sort of thing but I would hope that there is some level of parity between the ActionScript code that you can write for each of the targets; plus, as I've found, all that's needed for this construct to work 100% of the time is if we could specify the correct dependencies for the BinaryData class. > >> I’d really like to have the compiler delay loading of static consts and var to the very end of the loading. That would allow this pattern… > True, and possibly that could work, but surely it would also be allowed if we generated the line: > goog.addDependency('../../../org/apache/royale/utils/BinaryData.js', > ['org.apache.royale.utils.BinaryData'], [ 'org.apache.royale.utils.Endian', > 'org.apache.royale.utils.IBinaryDataInput', 'org.apache.royale.utils.IBinaryDataOutput']); > > So regardless of all the discussion, I would quite like to find out why the 'Endian' isn't listed as a dependency when all this html/JS stuff is generated... likewise some of the other constructs where something depends on something else (e.g. "is" is being used in the Namespace constructor, but I'm creating a Namespace prior to the Language.js file being loaded..) > > I guess we can't actually wait until the end of loading everything until we start to initialise things, unless we change how the JS code is output (i.e. to have a single static initialiser for the class which is run after everything is loaded, rather than declaring all the static properties with initialisation in-place..) > > Interesting problem! > > thanks > > Andrew > > > -----Original Message----- > From: Harbs [mailto:[email protected]] > Sent: 19 July 2018 14:19 > To: [email protected] > Subject: [EXTERNAL] Re: Query on Royale dependency generation > > If it’s in the big list, it should be initialized correctly. > > Are you initializing a BinaryData as a static const (or preinitialized var)? > > i.e. public static const DATA:BinaryData = new BinaryData() is a no-no. > > I’d really like to have the compiler delay loading of static consts and var to the very end of the loading. That would allow this pattern… > >> On Jul 19, 2018, at 4:06 PM, Frost, Andrew <[email protected]> wrote: >> >> Hi >> >> Yes (it's weird!) - okay so I just set the -remove-circulars option to true explicitly but still no change (from the default). >> >> If you look in your html file, do you get just: >> goog.addDependency('../../../org/apache/royale/utils/BinaryData.js', >> ['org.apache.royale.utils.BinaryData'], >> ['org.apache.royale.utils.IBinaryDataInput', >> 'org.apache.royale.utils.IBinaryDataOutput']); >> for the dependencies of it? >> >> In the big list of dependencies at the start, then yes Endian is showing up there. BinaryData isn't in that big list, because this is a dependency of another of my classes (which is in the list). It's constructor is being called because there's a static property being initialised i.e. >> static private var _bytes : BinaryData = new BinaryData(); >> >> >> From a bit more digging, the issue (whether it happens or not) seems to be due to the order in which the files are loaded; there can be a specific order be applied in terms of ensuring that the dependencies are loaded before the classes that require them, but there isn't a dependency link between BinaryData and Endian (as you can see from your compiled .js files). So without this dependency, it seems like luck? as to which order these go in? >> >> I've just created another project and this one works fine, but in the network info I can that Endian.js is being loaded prior to BinaryData.js. I can't see why this would be: it seems to just be down to the order that the big dependency list at the start of the html script is written. And I can't actually see why with one project, I'm ending up with one of our own classes early on in that list ... which means I currently have two solutions for fixing this: (a) add the endian dependency to the binarydata line in the html; (b) reorder the main list of dependencies at the top of the html to make endian appear earlier on. Neither of these are a good solution when this file is generated on every build!! >> >> So: am I right in thinking that the dependency list for the BinaryData.js should include the Endian one? i.e. this is wrong: >> goog.addDependency('../../../org/apache/royale/utils/BinaryData.js', >> ['org.apache.royale.utils.BinaryData'], >> ['org.apache.royale.utils.IBinaryDataInput', >> 'org.apache.royale.utils.IBinaryDataOutput']); >> >> If so, then that needs to change (somehow) and it would then solve my problem... >> >> thanks >> >> Andrew >> >> >> -----Original Message----- >> From: Harbs [mailto:[email protected]] >> Sent: 19 July 2018 13:32 >> To: [email protected] >> Subject: [EXTERNAL] Re: Query on Royale dependency generation >> >> The Endian dependency should be added to the main application dependency. The entire list should be included there. (I have 872 dependencies listed.) ‘org.apache.royale.utils.Endian’ is one of them. >> >> I am also using URLStream and URLBinaryLoader which list Endian as a dependency, so it’s possible it’s being pulled from there. I’m not sure I have an application which uses BinaryData without one of those. >> >>> On Jul 19, 2018, at 3:19 PM, Harbs <[email protected]> wrote: >>> >>> That’s weird. I believe the default is false (although I think the default should be true). >>> >>> Try setting -remove-circulars to true. I think that should resolve it. >>> >>> Yes. I’m using BinaryData extensively. >>> >>>> On Jul 19, 2018, at 3:15 PM, Frost, Andrew <[email protected]> wrote: >>>> >>>> Hi >>>> >>>> I'm not using it on my build command line, i.e. it's set as the default 'true'. >>>> >>>> If I do set it to 'false' then I get what I would expect from the code, i.e. it ignores the interfaces and just outputs the discovered dependencies - including the google one: >>>> goog.addDependency('../../../org/apache/royale/utils/BinaryData.js', >>>> ['org.apache.royale.utils.BinaryData'], ['goog.DEBUG', >>>> 'org.apache.royale.utils.Endian']); >>>> >>>> But now I get hundreds of errors from the browser's JS engine e.g. >>>> IStatesObject.js:42 Uncaught TypeError: Cannot read property >>>> 'IEventDispatcher' of undefined at IStatesObject.js:42 >>>> (anonymous) @ IStatesObject.js:42 >>>> IUIBase.js:58 Uncaught TypeError: Cannot read property >>>> 'IEventDispatcher' of undefined at IUIBase.js:58 >>>> (anonymous) @ IUIBase.js:58 >>>> IBeadModel.js:38 Uncaught TypeError: Cannot read property >>>> 'IEventDispatcher' of undefined at IBeadModel.js:38 >>>> (anonymous) @ IBeadModel.js:38 >>>> Event.js:33 Uncaught TypeError: Cannot read property 'Event' of >>>> undefined at Event.js:33 >>>> (anonymous) @ Event.js:33 >>>> base.js:2484 Uncaught TypeError: Cannot read property 'prototype' of >>>> undefined at Object.goog.inherits (base.js:2484) at UIBase.js:44 >>>> goog.inherits @ base.js:2484 >>>> (anonymous) @ UIBase.js:44 >>>> base.js:2484 Uncaught TypeError: Cannot read property 'prototype' of >>>> undefined at Object.goog.inherits (base.js:2484) at >>>> eventtarget.js:96 goog.inherits @ base.js:2484 >>>> (anonymous) @ eventtarget.js:96 >>>> base.js:2484 Uncaught TypeError: Cannot read property 'prototype' of >>>> undefined at Object.goog.inherits (base.js:2484) at >>>> HTMLElementWrapper.js:28 ..... >>>> >>>> >>>> I'm assuming I'm not meant to be editing the html dependency list manually (there's another one to change, Namespace.js has a dependency on Language.js due to the use of "is") so unless something here is project-specific that's changing how it's outputting/parsing the dependencies, I'm not sure what's up. >>>> >>>> If anyone has a project that uses BinaryData, are they able to check >>>> what they see in the generated HTML for that one, to see whether >>>> it's just me who doesn't have the Endian dependency added..? FWIW >>>> I've been trying both with 0.9.2 downloaded via NPM, and the latest >>>> develop branch (well perhaps a week out of date now..) >>>> >>>> thanks >>>> >>>> Andrew >>>> >>>> >>>> -----Original Message----- >>>> From: Harbs [mailto:[email protected]] >>>> Sent: 19 July 2018 12:18 >>>> To: [email protected] >>>> Subject: [EXTERNAL] Re: Query on Royale dependency generation >>>> >>>> Are you using the -remove-circulars compiler option? >>>> >>>>> On Jul 19, 2018, at 1:05 PM, Frost, Andrew <[email protected]> wrote: >>>>> >>>>> Hi guys >>>>> >>>>> I'd been getting an error when running a simple Royale application: >>>>> Uncaught TypeError: Cannot read property 'BIG_ENDIAN' of undefined >>>>> at new org.apache.royale.utils.BinaryData (BinaryData.js:28) >>>>> >>>>> the line in question is from the constructor: >>>>> org.apache.royale.utils.BinaryData = function(bytes) { bytes = >>>>> typeof bytes !== 'undefined' ? bytes : null; this._endian = >>>>> org.apache.royale.utils.Endian.BIG_ENDIAN; >>>>> and "Endian" is undefined. >>>>> >>>>> After a little digging I found this is because the BinaryData object is being constructed without the JS engine having knowledge of the "Endian" class: something went wrong with the google dependency thing. In my generated html page I have a line: >>>>> goog.addDependency('../../../org/apache/royale/utils/BinaryData.js' >>>>> , ['org.apache.royale.utils.BinaryData'], >>>>> ['org.apache.royale.utils.IBinaryDataInput', >>>>> 'org.apache.royale.utils.IBinaryDataOutput']); >>>>> and if I change this to: >>>>> goog.addDependency('../../../org/apache/royale/utils/BinaryData.js' >>>>> , ['org.apache.royale.utils.BinaryData'], >>>>> ['org.apache.royale.utils.IBinaryDataInput', >>>>> 'org.apache.royale.utils.IBinaryDataOutput', >>>>> 'org.apache.royale.utils.Endian']); >>>>> then it works. >>>>> >>>>> Looking at where this comes from in the compiler: >>>>> compiler-jx/src/main/java/org/apache/royale/compiler/internal/graph >>>>> / Go ogleDepsWriter.java function "generateDeps" is creating these >>>>> lists, and if the "removeCirculars" value is true (which it is by >>>>> default unless changed on the command-line) then we add dependencies for the interfaces that we implement (gd.fileInfo.impls) and any static dependencies (gd.fileInfo.staticDeps) but we don't add the actual dependencies that were calculated (gd.deps or gd.fileInfo.deps - both of these contain the Endian definition). >>>>> >>>>> So I can fix my project by updating the compiler to do: >>>>> if (gd.fileInfo.deps != null) >>>>> deps.addAll(gd.fileInfo.deps); and then it works: the >>>>> generated line though is: >>>>> goog.addDependency('../../../org/apache/royale/utils/BinaryData.js' >>>>> , ['org.apache.royale.utils.BinaryData'], ['goog.DEBUG', >>>>> 'org.apache.royale.utils.Endian', >>>>> 'org.apache.royale.utils.IBinaryDataInput', >>>>> 'org.apache.royale.utils.IBinaryDataOutput']); >>>>> >>>>> >>>>> So my questions: >>>>> >>>>> 1. where is the fault here? Am I right in thinking that there's a >>>>> missing set of dependencies that need to also be added per the above snippet, or should the Endian definition be listed as a dependency in the gd.fileInfo.staticDeps list (which is null for me) 2. presumably we don't want "goog.DEBUG" to end up in the dependency list: is there a sensible way of getting rid of this (or should we just manually filter out anything starting "goog.")? >>>>> 3. if we should be adding these dependencies separately, is there a preference for "gd.deps" vs "gd.fileInfo.deps"? >>>>> >>>>> thanks >>>>> >>>>> Andrew >>>>> >>>>> >>>> >>> >> >
