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