Sorry, yes, of course it's false. Haven't had my morning tea yet. -- Josh Tynjala Bowler Hat LLC <https://bowlerhat.dev>
On Thu, Feb 20, 2020 at 7:17 AM Josh Tynjala <[email protected]> wrote: > Do you mean -rename-public-vars=true instead of false? False should > preserve the original behavior. > > -- > Josh Tynjala > Bowler Hat LLC <https://bowlerhat.dev> > > > On Thu, Feb 20, 2020 at 1:44 AM Alex Harui <[email protected]> > wrote: > >> 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 >> > > > >> > > > >> >>> >> > > > >> >> >> > > > >> >> > > > >> >> > > > >> > > > >> > > > >> > > >> > > >> > > >> > >> > >> > >> >> >>
