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 > > >> >>> > >> >> > >> > >> > > >
