That's a good point about checking whether Flex could do that. I think that the compiler may need to know the state names at compile-time, but binding is generally considered a run-time thing.
-- Josh Tynjala Bowler Hat LLC <https://bowlerhat.dev> On Wed, Feb 5, 2020 at 12:54 PM Greg Dove <[email protected]> wrote: > Did that work in Flex for states? I seem to remember issues with trying to > use constants with states, although maybe it was with other parts of how > states are used. > > The issue is that you can have things like: > > <MyComp x.All="25" x.Completed="100" /> (I have never tried this with > uppercase first letter in state names, but presume it works) > > And I don't think there is any way to get the state variations from the > constant declarations in that case. > > For constant bindings in general...I can try to find some old code I worked > on for converting static constant bindings with primitive values like that > to inlined value assignments (which removes them from the binding data and > puts them into the normal mxmldescriptor startup assignments for the > instances). > > > > > On Thu, Feb 6, 2020 at 9:38 AM Josh Tynjala <[email protected]> > wrote: > > > My initial implementation will probably all or nothing, in terms of > > renaming, but we can revisit after to add more fine-grained control, if > > necessary. > > > > I'll try to take a look at that constant issue when I get a chance. > > > > -- > > Josh Tynjala > > Bowler Hat LLC <https://bowlerhat.dev> > > > > > > On Wed, Feb 5, 2020 at 12:23 PM Harbs <[email protected]> wrote: > > > > > I hope there will be an option to prevent specific variables. > > > > > > The obvious ones would be ones used in MXML. > > > > > > Other candidates would be classes which correspond to JSON so the > classes > > > can be constructed with bracket notation. I’m not sure of the best way > to > > > declare a class for that. Maybe abusing “dynamic” classes? > > > > > > FWIW, I also noticed a bug recently that I didn’t log: > > > > > > In todomvc there’s the following in the router branch: > > > > > > <j:states> > > > <js:State name="All"/> > > > <js:State name="Active"/> > > > <js:State name="Completed"/> > > > </j:states> > > > > > > I tried using constants like so, and it failed. (At runtime the values > > > were “ALL”,”ACTIVE_FILTER” and “COMPLETED_FILTER”.) > > > <j:states> > > > <js:State name="{TodoModel.ALL}"/> > > > <js:State name="{TodoModel.ACTIVE_FILTER}"/> > > > <js:State name="{TodoModel.COMPLETED_FILTER}"/> > > > </j:states> > > > > > > > > > > On Feb 5, 2020, at 9:41 PM, 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://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 > > > >>> > > > >>>>>>>> > > > >>>>>>> > > > >>>>> > > > >>>>> > > > >>> > > > >>> > > > >>> > > > >> > > > >> > > > >> > > > > > > > > >
