On 2/20/20, 6:50 AM, "Carlos Rovira" <[email protected]> wrote:
Hi Alex,
I think this should transparent for the final user and not need to add a
compiler option, just configure all the strings that can cause problems,
since for what I understand the problem, we can know what strings are
problematic, right?
You don't know what strings are problematic when compiling the application.
You only find out when you later compile the module. The only solution I've
thought of so far is to allow a list of reserved names when compiling the
application so avoid collisions when compiling the module. You "can" add code
to the application to reserve that string but that adds weight to the
application, and you have to add that code in a way it is not seen as dead code
and removed.
TourDeFlexMigration is currently not building for this very reason. I might
try to find a pattern to add code to the application, if I find some time, but
I think reserving a list of names is probably best.
-Alex
Thanks
El jue., 20 feb. 2020 a las 10:44, Alex Harui (<[email protected]>)
escribió:
> 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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124083456&sdata=rPzChKqkUGUPyf94NhLBfIQRFmxGnTuQ6%2FQOi1ux2J0%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=pA01nbT%2BFHtnISBm9Fn1cq2HMR1jjrje9VWHKQfQ7vc%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=pA01nbT%2BFHtnISBm9Fn1cq2HMR1jjrje9VWHKQfQ7vc%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=pA01nbT%2BFHtnISBm9Fn1cq2HMR1jjrje9VWHKQfQ7vc%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=0e5p3qrPLiRK1BZouGpU3Dnj70LI4VrhKNZs23R5eGM%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=0e5p3qrPLiRK1BZouGpU3Dnj70LI4VrhKNZs23R5eGM%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=s2qIjF5F3fJcvnIWOBnTOKHG3FTo6i1xaUJ04%2FhgShY%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%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=pA01nbT%2BFHtnISBm9Fn1cq2HMR1jjrje9VWHKQfQ7vc%3D&reserved=0
> > > >
> > > > >> >>>
> > > > >> >>
> > > > >>
> > > > >>
> > > >
> > > >
> > > >
> > >
> > >
> > >
> >
> >
> >
>
>
>
--
Carlos Rovira
https://nam04.safelinks.protection.outlook.com/?url=http%3A%2F%2Fabout.me%2Fcarlosrovira&data=02%7C01%7Caharui%40adobe.com%7C8837468e7a2e4344564b08d7b6142ec7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637178070124093456&sdata=JCe1NkhbCbHDrfEYvIr2oBt6BKdeTIRL%2F66510k%2F%2F5Q%3D&reserved=0
