I turned on -remove-circulars by default for now so I could push other
changes. ASDoc now appears to be working in js-release. I am going to
push the js-release version to the website.
-Alex
On 2/17/18, 8:21 PM, "Alex Harui" <aha...@adobe.com.INVALID> wrote:
>
>
>On 2/17/18, 9:00 AM, "Gabe Harbs" <harbs.li...@gmail.com> 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 <aha...@adobe.com.INVALID>
>>>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
>>>
>>>
>>>
>>
>
