This change is breaking some modules when -rename-public-vars=false. Let's say the main app has some method "foo" that gets renamed. It might accidentally get a minified name that coincides with a public var in the module. For tour de flex, some API gets the minified name "UP". A module has code that references Keyboard.UP.
In Pashmina's app, some API gets a minified name like "vs" and the module has a property "vs" (for a ViewStack). We try to allow renaming of APIs in modules for size savings. The list of renames in the main app is output to a file and read into the compiler when compiling the module so it starts with the same renaming map. One thought I had about solving this is to add another compiler option that allows a list of other names to not rename. So for the "UP" scenario, I would compile the main app with, say, -forbidden-minified-names=UP. Folks can probably work around the problem by using Keyboard.UP in the main app, but that bloats the main app. The compiler can already read a file of rename maps, so we could just use that, but I think a simpler command-line list will be more convenient. Thoughts? -Alex On 2/5/20, 12:18 PM, "Josh Tynjala" <[email protected]> wrote: Yeah, I'll make sure that users can control whether renaming happens or not. -- Josh Tynjala Bowler Hat LLC <https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=a5evE514IT0GdVvL7w5IbOCcnyb%2FCwCqgxhOIlJsFvM%3D&reserved=0> On Wed, Feb 5, 2020 at 11:51 AM Alex Harui <[email protected]> wrote: > Great work, Josh! > > I have to say that the objectProperty output was adding pain to debugging > so looking forward to that going away. I'm assuming there will be compiler > options/directives to control renaming? I think there are some scenarios > where it is safe to have public variables renamed. > > Thanks, > -Alex > > On 2/5/20, 11:44 AM, "Josh Tynjala" <[email protected]> wrote: > > 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://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=a5evE514IT0GdVvL7w5IbOCcnyb%2FCwCqgxhOIlJsFvM%3D&reserved=0 > > > > > 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%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=a5evE514IT0GdVvL7w5IbOCcnyb%2FCwCqgxhOIlJsFvM%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%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=a5evE514IT0GdVvL7w5IbOCcnyb%2FCwCqgxhOIlJsFvM%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%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=O5PMzmZaDJnnWfkJL1EQMlGudNjREP88VIbImidkXtw%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%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=O5PMzmZaDJnnWfkJL1EQMlGudNjREP88VIbImidkXtw%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%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=M%2BEvAma5g8ufQL0Nf2Krb3hrr2ERTHhdsofqVUhm7f4%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%7C8dcbb865725d4151564408d7aa7893c9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637165307165113089&sdata=a5evE514IT0GdVvL7w5IbOCcnyb%2FCwCqgxhOIlJsFvM%3D&reserved=0 > > > > > > >> >>> > > > >> >> > > > >> > > > >> > > > > > > > > > > > > > > > > > >
