On 2/17/18, 9:00 AM, "Gabe Harbs" <[email protected]> wrote:
>FWIW, none of the apps I built to date compiled correctly without
>-remove-circulars.
>
>I’m not sure I completely understand the issue. Will your proposed
>solution make minified apps fatter, or just the debug apps?
Good question. I think you might be right that the optimizer will kick
out those classes anyway. Those data classes will likely just be used to
make the closure compiler happy and prevent renaming.
>
>Can you point me to where in the ASDoc code the problem is so In can
>understand the problem better?
ASDocModel manages the JSON results. What is in the repo doesn't contain
the ValueObjects I'm now working with, but I have a JSONReviver class that
converts JSON to VOs on the fly. The JSON works from registered
[RemoteClasses] similar to how AMF works. That means, if I have a class
called ASDocClass that represents the ASDoc for a Class, there is no code
my local version of ASDocModel that calls "new ASDocClass". Instead, it
just does:
var data:ASDocClass = app.reviver.parse(app.service.data) as ASDocClass;
Then, when optimizing "data.members", if the compiler has already removed
the goog.require('ASDocClass') then the Closure Compiler might rename
members to "xy".
The compiler currently tries to detect how you used a class in a file to
decide whether it needs to be goog.require'd or not. The current
ASDocModel code also has an "@flexjsignorecoercion ASDocClass" so the JS
output of the line above is:
var /** @type {ASDocClass} */ data =
this.app.reviver.parse(this.app.service.data);
Since this code can run without needing an ASDocClass definition, the
current compiler removes the goog.require. This helps reduce the number
of circularities in Flex since simple parent/child or bead/strand always
always has at least one class in the pair reference the other class as
only a type. For example:
interface IParent {
var child:IChild;
}
interface IChild {
var parent:IParent;
}
The JS output is, more or less:
goog.provide('IParent');
goog.require('IChild');
IParent = function();
/** @type {IChild} */
IParent.prototype.IChild;
goog.provide('IChild');
goog.require('IParent');
IChild = function();
/** @type {IParent} */
IChild.prototype.IParent;
Again, you can see that IParent doesn't have to exist at runtime to make
IChild happy and vice versa. But the Closure Compiler will see the
goog.requires as a circularity, so that's why the current Royale Compiler
found a way to remove them. I don't know a way to detect that a
goog.require is only used for type-checking and not later used to prevent
renaming. So the change I made was to turn off the pruning of
goog.requires. I think this will help prevent renaming in more places and
make the js-release version work more often. But now, every app will use
IParent and IBead and result in circularities from Closure Compiler.
The reason -remove-circulars is not on by default is that some folks
wanted to be able to write apps that don't have circularities so they want
Closure Compiler to complain so they can fix their code by breaking the
circularities. Also note that -remove-circulars also removes
goog.requires and thus still presents a risk that closure compiler will
rename something it shouldn't. -remove-circulars chooses to remove
goog.requires that have already been seen by a dependency walk starting
with the main class. The circularity removal code uses a different
criteria for deciding whether to remove a goog.require. It will remove a
goog.require unless it sees it is used in a static variable initializer.
So, there is a chance that as we continue to debug js-release versions we
will see that we must break circularities the theoretical way instead of
by using -remove-circulars. We might just be getting lucky so far.
I am devoting time to this now not only to get ASDoc running but to get a
better understanding of why js-release versions don't work so we know what
to look for when we debug really big apps some day.
What I'm working on now is changing over some "public var foo" to
getter/setters in order to address renaming issues for properties
referenced by MXML code. I'm probably going to add a warning to the
compiler on every use of "public var" to help us sweep the code and
convert to getter/setter to ward off renaming issues later.
HTH,
-Alex
>
>Harbs
>
>> On Feb 17, 2018, at 7:40 AM, Alex Harui <[email protected]>
>>wrote:
>>
>> Hi Folks,
>>
>> I've spent the last day or so trying to get ASDoc to run when minified.
>> I
>> created a JSON to ValueObjects utility which helped a lot. It still
>>isn't
>> completely running, but it appears that we need to stop pruning classes
>> that are only used in type annotations. This will make most apps a
>>little
>> fatter, but also require all apps to use remove-circulars.
>>
>> What do folks think about that? The compiler used to prune classes
>>from a
>> file's goog.requires list that were never instantiated or type-checked
>>in
>> that file. So, for ASDoc, the ASDocClass ValueObject that represents
>>the
>> JSON data for the ASDoc is never instantiated by the class that consumes
>> it. The instances are created by JSON. The ASDocClass is only
>>mentioned
>> in JSDoc to declare some variable as being of type ASDoc. That enabled
>>us
>> to remove that class from the requires list since that ASDocClass is
>>never
>> referenced by operational code. Removing that goog.require helped
>> eliminate circular goog.require references that the closure compiler
>> disallows. That kicked HTMLElementWrapper out of the app for example.
>> But it appears that by removing that goog.require, the compiler didn't
>> know that the ASDocClass properties were exported and it renamed some of
>> them resulting in problems in js-release. So by leaving more
>> goog.requires in the code we get better support against renaming, but we
>> also bring back more circularities and now even HelloWorld would need
>> -remove-circulars.
>>
>> An alternative that would allow a few more apps to not need
>> remove-circulars is to modify a few of our examples and classes to
>> eliminate circularities. HelloWorld only has 3. But 2 of them are
>> IParent/IChild and IStrand/IBead. It seems "right" to have IParent
>> reference IChild and vice versa. Same for IStrand and IBead. I'm not
>> quite sure what the answer is. Maybe IParent can reference IChild, but
>> IChild does not reference its IParent. I guess you could make an
>> IParentedChild interface with the parent property and concrete children
>> implement both IChild and IParentedChild. That seems wrong and overly
>> complicated.
>>
>> Yet another option, which is recommended by some folks in the Closure
>> Compiler community but doesn't fit well with the ActionScript language
>>is
>> to define both IChild and IParent in the same file. I do not want to
>> spend the time modifying the compiler for that. It might be practical
>>to
>> merge the files as part of the Ant and Maven builds. But again, that
>>will
>> take time as well.
>>
>> That said, requiring remove-circulars always doesn't seem quite right
>> either.
>>
>> Thoughts?
>> -Alex
>>
>>
>>
>
