Thank you for the tips, Alex. Much appreciated. With your help, I've determined how to use Closure compiler's Java API to prevent the renaming of a specific public variable that has not been @export-ed. Now, I should be able to expand this prototype to a full version that prevents the renaming of all public variables.
-- Josh Tynjala Bowler Hat LLC <https://bowlerhat.dev> On Sun, Jan 19, 2020 at 4:58 PM Alex Harui <[email protected]> wrote: > In response to your prior post, the reason I am saying it removes control > is because I didn't see any option to not have the compiler output > goog.reflect.objectProperty and I'm not clear everyone will want/need it. > > Regarding how to control Closure Compiler's renaming, the details might be > changing because I believe I saw that Google refactored the renamer. At a > high-level, you probably know most of this, but for other folks reading, > the Closure Compiler is a set of Java Classes that form a series of > Compiler Passes. Each Pass takes information (sometimes source, sometimes > the AST, sometimes other information, and modifies the AST. IIRC, a final > pass generates the output. There might be more than one pass for output. > > The renaming pass we currently use can output as well as accept a file of > rename mappings. I’m confident we can subclass or modify and replace the > renaming pass and feed it a set of mappings. If you look in the > royale-compiler source, we've already done this for some other passes. > Look through the Closure compiler source for what happens to the compiler > options: > > --variable_map_input_file > --property_map_input_file > > You can build examples/mxroyale/TourDeFlexModules which outputs these > files to see what is in them. > > > We should also see if we can agree on the scenarios and likelihood of > property access "by name". I can quickly think of: > > A) MXML setting properties by reference (high usage) > B) MXML setting properties by value (high usage) > C) Serializers/Deserializers (moderate usage) > D) [] bracket access by Literal (occasional usage) > E) [] bracket access by String Variable (occasional usage) > F) [] bracket access by Expression (infrequent usage) > > Exports can solve A. The compiler can easily detect D & E and prevent > renaming. For C, we "could" autogenerate exports for any classes with > [RemoteClass] metadata or autogenerate getter/setters. > > To me, the only difficult case is F and it will rarely happen. Maybe we > can detect those and warn on that. > > Of course, I could be wrong... > -Alex > > > On 1/17/20, 10:08 AM, "Josh Tynjala" <[email protected]> wrote: > > Comments inline. > > On Thursday, January 16, 2020, Alex Harui <[email protected]> > wrote: > > Maybe we should start by agreeing on facts and then goals and then > discuss solutions. > > Yes, I think that's a good place to start. > > > > > Here are some facts that come to mind, not a complete list. > > > > 1) An export does not prevent renaming. It builds an alias. All > references within the set of sources to be minified are renamed. > > Agreed. > > > 2) Closure's export mechanism only works on non-scalars (Object, > Arrays, > Functions) and not Number, String, Boolean because non-scalars are > pass-by-reference instead of pass-by-value > > Agreed. > > > 3) The Closure Compiler is open source and designed to be extended > > Agreed. > > > 4) Use of goog.reflect.objectProperty is not necessarily the only > way to > control renaming. It is the way recommended by Google for those who > can't > extend the compiler. We are not constrained to modify our output > because > we have control over the compiler. > > Could you share some details how we might have more control over > Closure > compiler's renaming? It sounds like you know, at least somewhat, how > to use > its lower-level Java APIs, but you've never shared the details when > you've > mentioned them in this thread or in the past. > > I should add that I've personally tried to research this topic myself, > but > I had a very hard time finding any information that wasn't just someone > explaining to a JS developer that they needed to modify their JS code. > I > eventually couldn't justify spending more time to keep looking. > > > 5) The compiler knows things about how properties were accessed. > That > information is lost in the output in many cases. Therefore, it should > be > better to inform the Google minifier directly from the Royale compiler, > instead of leaving hints in the output. > > Agreed. I'm personally not fully convinced that the Royale compiler has > enough information for dynamic stuff (like for serialization with type > Object), but that may be due to ignorance about Closure compiler's > capabilities. Even without knowing how it works, I can imagine how it > might > be relatively easy to prevent renaming of public variables, but the > dynamic > stuff is trickier. For the dynamic stuff, maybe it's just a matter of > Closure detecting when a variable is typed as Object, and then it can > switch to ["string"] syntax on its own (instead of us doing it in the > debug > build, like with -js-dynamic-access-unknown-members). > > > 7) We are pretty close to allowing renaming across modules. It was > working for a while, but a scenario popped up that isn't currently > handled. We can pre-load the Closure renamer with a name map. > > I haven't looked in detail at the module implementation and don't plan > to, > but I understand it well enough at a high level to say "agreed" here > too > > > > > These are hypotheses, and not proven facts. > > 8) The big gain from not exporting everything is in dead code removal > instead of shorter variable names > > Agreed, personally. It seems like others have expressed interest in > both, > though. I hope that they'll be willing to prioriitze dead code removal, > since it will probably have the bigger impact (my own tests removing > @export have been promising in this regard). > > > 9) Renaming can complicate and slow serialization/deserialization > > Agreed, and this is the harder portion to get working, I think. > > However, if release builds didn't rename public variables, and also > didn't > rename dynamic accesses, that would remove my biggest frustration with > how > ActionScript works in Royale/JS compared to SWF. If both kept their > original names, things that feel broken today would "just work" again. > > > > > IMO, we want to be heading in the direction of A) allowing control > over > what gets renamed > > Agreed, but as I said before, I think that dead code removal will have > more > impact than control over renaming, so if it's one or the other, I'm > okay > with no control over renaming. > > > B) capturing information from the compiler, > > C) controlling the set of renames and exports directly, not through > the > output. > > Agreed, being able to pass information Closure compiler on the Java > side > would be better. than through the JS output > > > > > > My 2 cents, > > -Alex > > > > > > On 1/16/20, 2:48 PM, "Josh Tynjala" <[email protected]> > wrote: > > > > Some additional context, if anyone is interested. > > > > At the request of Harbs, I am currently investigating how we > might > remove > > @export from our generated JS code to improve the minimization > even > more. > > When I modified the compiler to skip emitting @export in some > places, > a > > release build of TourDeJewel was initially broken. When I added > > goog.reflect.objectProperty(), not only did it fix setting public > variables > > in MXML, it also made that release build of TourDeJewel start > working > again. > > > > -- > > Josh Tynjala > > Bowler Hat LLC < > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7C1e9c2c4ae9d049b2896708d79b7843c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637148813154391097&sdata=8T%2B4UIVZrakKMBkAs2qOcwalYaCVJuMxHMKPYTnbvxM%3D&reserved=0 > > > > > > > > On Thu, Jan 16, 2020 at 12:59 PM Josh Tynjala < > [email protected]> > > wrote: > > > > > Thank you, Harbs! Wrapping the variable name in a > > > goog.reflect.objectProperty() call works perfectly. This is > exactly > why I > > > started this thread, to see if anyone could suggest possible > alternatives. > > > > > > Thankfully, we can keep the same simple data structure as > before, > and my > > > initial proposal with functions can be forgotten. In a release > build, I can > > > see that goog.reflect.objectProperty() calls are replaced by a > simple > > > string literal (containing the minified variable name), so we > don't > have to > > > worry about extra performance impact. > > > > > > -- > > > Josh Tynjala > > > Bowler Hat LLC < > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7C1e9c2c4ae9d049b2896708d79b7843c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637148813154391097&sdata=8T%2B4UIVZrakKMBkAs2qOcwalYaCVJuMxHMKPYTnbvxM%3D&reserved=0 > > > > > > > > > > > On Wed, Jan 15, 2020 at 8:32 PM Harbs <[email protected]> > wrote: > > > > > >> Sounds good! > > >> > > >> > > >> > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgoogle%2Fclosure-compiler%2Fwiki%2FType-Based-Property-Renaming&data=02%7C01%7Caharui%40adobe.com%7C1e9c2c4ae9d049b2896708d79b7843c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637148813154391097&sdata=IzzY7%2BhSQ9iQr9GIKd16XD%2FfGwxB50aN6J0Mzb%2BEcWA%3D&reserved=0 > > >> < > > >> > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgoogle%2Fclosure-compiler%2Fwiki%2FType-Based-Property-Renaming&data=02%7C01%7Caharui%40adobe.com%7C1e9c2c4ae9d049b2896708d79b7843c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637148813154391097&sdata=IzzY7%2BhSQ9iQr9GIKd16XD%2FfGwxB50aN6J0Mzb%2BEcWA%3D&reserved=0 > > >> > > > >> > > >> The function seems to be goog.reflect.objectProperty() > > >> > > >> I’m not sure exactly how it works though. > > >> > > >> > On Jan 16, 2020, at 1:37 AM, Greg Dove <[email protected] > > > wrote: > > >> > > > >> > actually just as another fyi, Harbs pointed out some > intriguing > goog > > >> > methods recently - I don't have an immediate reference to it > sorry. One > > >> of > > >> > those seemed to allow for access to renamed names by > wrapping the > > >> original > > >> > names in a 'magic' method that presumably GCC recognises > (but > presumably > > >> > returns the name unchanged in debug mode) > > >> > > > >> > > > >> > On Thu, Jan 16, 2020 at 12:33 PM Greg Dove < > [email protected]> > wrote: > > >> > > > >> >> reflection data has similar stuff to support release mode > get/set for > > >> >> public vars. > > >> >> > > >> >> I did not look at MXML startup assignments like this, but > it > sounds > > >> good > > >> >> to me. I don't know if it makes sense, but considering > this is > just > > >> startup > > >> >> assignments, could one function combine all of the startup > assignments > > >> (in > > >> >> the same sequence as before)? > > >> >> > > >> >> > > >> >> On Thu, Jan 16, 2020 at 12:23 PM Josh Tynjala < > > >> [email protected]> > > >> >> wrote: > > >> >> > > >> >>> According to the commit linked below, the > -warn-public-vars > compiler > > >> >>> option > > >> >>> was added because setting a public var in MXML does not > currently work > > >> >>> properly in a release build. > > >> >>> > > >> >>> > > >> >>> > > >> > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fapache%2Froyale-compiler%2Fcommit%2Feed5882ba935870a98ba4fe8cbf499e5d8344f60&data=02%7C01%7Caharui%40adobe.com%7C1e9c2c4ae9d049b2896708d79b7843c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637148813154391097&sdata=aA3JAnaQuU1GrF%2B4vmTo1Lhn38gDM4UtG2%2FdbmwBnjQ%3D&reserved=0 > > >> >>> > > >> >>> In other words, this MXML code won't work if it's a public > variable > > >> and > > >> >>> not > > >> >>> a setter: > > >> >>> > > >> >>> <Component publicVar="value"/> > > >> >>> > > >> >>> For reference, the compiler currently writes the name of > the > public > > >> >>> variable as a string to the generated JS, like this: > > >> >>> > > >> >>> var data = [ > > >> >>> Component, > > >> >>> 1, > > >> >>> 'publicVar', > > >> >>> true, > > >> >>> 'value' > > >> >>> ] > > >> >>> > > >> >>> At runtime, it interprets this array of properties, and > basically runs > > >> >>> code > > >> >>> like this: > > >> >>> > > >> >>> comp['publicVar'] = 'value'; > > >> >>> > > >> >>> Since Closure compiler rewrites variable names during the > minification > > >> >>> process, this code keeps using the original name, but > other > code in > > >> the > > >> >>> app > > >> >>> might start looking for a shorter variable name like "uB". > This is the > > >> >>> failure that we're warning about. > > >> >>> > > >> >>> I propose updating the code generated by the compiler to > something > > >> like > > >> >>> this instead: > > >> >>> > > >> >>> var data = [ > > >> >>> Component, > > >> >>> 1, > > >> >>> function(){ this.publicVar=true } > > >> >>> ] > > >> >>> > > >> >>> At runtime, the class that interprets MXML data will > detect the > > >> function > > >> >>> and call it like this: > > >> >>> > > >> >>> func.apply(comp); > > >> >>> > > >> >>> Because this new code will no longer use a string, > Closure can > > >> rewrite the > > >> >>> property name with its minified version, just like in > other > parts of > > >> the > > >> >>> app, and we'll no longer need to warn on declarations of > public > > >> variables. > > >> >>> > > >> >>> I have a working prototype for primitive values, like > String, > > >> Boolean, and > > >> >>> Number. Objects and Arrays follow a different path in the > MXML > data > > >> >>> interpreter, but I don't see why I wouldn't be able to > handle > those > > >> with a > > >> >>> similar approach. > > >> >>> > > >> >>> Thoughts? > > >> >>> > > >> >>> -- > > >> >>> Josh Tynjala > > >> >>> Bowler Hat LLC < > > https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7C1e9c2c4ae9d049b2896708d79b7843c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637148813154391097&sdata=8T%2B4UIVZrakKMBkAs2qOcwalYaCVJuMxHMKPYTnbvxM%3D&reserved=0 > > > > >> >>> > > >> >> > > >> > > >> > > > > > > > > >
