> I would like to know what got rolled back. IOW, what is the status now?
Is the compiler back to where it was maybe two years ago where we warn on
public vars, but the output is not made more verbose? Or what?
I'm not sure what "two years ago" refers to exactly. All of the commits
that I rolled back were from this year, 2020.
But to answer what I rolled back, yes, it will warn on public vars again,
and the names of properties in the MXML descriptor are back to simple
strings. No more use of goog.reflect.objectProperty() because I realized
that it would only work in some situations.
I'm currently looking into a way to include a function with the MXML
descriptor that can be used to set the property, even if it has been
renamed. An evolution of what I proposed a couple of months ago. However, I
plan to keep both the string name and the value inside the descriptor so
that it does not affect your debugging approach.
A single property in the MXML descriptor looks like this now:
[
"someProperty",
true,
123.4
]
It might look something like this after my change:
[
"someProperty",
function(value) { this.someProperty = value; },
true,
123.4
]
If the function exists, it will be called like this (where host is the
component/object being created):
var func = descriptor[i];
func.call(host, value);
This will ensure that someProperty can be renamed, and all of the original
data is preserved for debugging.
I should be able to make the compiler smart enough to only include the
function where necessary (public vars) and omit it when not necessary
(public setters) to save on file size and make it PAYG. I would, of course,
also add a compiler option to include no functions in the descriptor (in
other words, keep the existing behavior).
> Also note that module support currently allows renaming, although Josh's
original post got me wondering if I missed something along the way. Google
Closure was taking a rename map and seemed to be using it.
I mentioned this in my original post. Closure's ability to use these rename
maps is intended specifically for compiling multiple projects together,
exactly how we are using it for modules. There are comments in the code
that explicitly say that the rename map should not be used as a way to
prevent things from being renamed, and that the rename maps are considered
by Closure to be suggestions only, and they may be ignored in some cases.
--
Josh Tynjala
Bowler Hat LLC <https://bowlerhat.dev>
On Thu, Mar 26, 2020 at 10:47 AM Alex Harui <[email protected]>
wrote:
> I'm also sorry to hear. I did not pay much attention to this effort,
> other than complaining that we should not make our debug output hard to
> work with in order to support such an effort, because it really did make
> the kind of debugging I do much more painful.
>
> I would like to know what got rolled back. IOW, what is the status now?
> Is the compiler back to where it was maybe two years ago where we warn on
> public vars, but the output is not made more verbose? Or what?
>
> We might be able to enumerate where the framework relies on dynamic
> access, but it will be much harder, without control-flow analysis, to know
> where the application code relies on dynamic access.
>
> Without thinking about it too much, I would think we can define some
> scenarios, represent those scenarios as compiler options, and make more
> people happy. But it could be a lot of work and I don't have the cycles
> for it.
>
> Some examples of a scenarios that could be output options:
> 1) I am not using binding or any other dynamic access in my app. Remove
> all exports and I'll debug the resuls and add directives to fix the few
> things that need fixing
> 2) I am not using modules, so don't worry about any other compilation
> accessing my APIs
> 3) I am using modules, and here are the classes that are referenced by the
> modules
>
> There is also another dimension of ways to prevent renaming:
> A) Please convert all public vars to getter/setter
> B) Please convert all public var access to dynamic access
> C) Please inject anti-renaming stubs for all dynamic access
>
> Also note that module support currently allows renaming, although Josh's
> original post got me wondering if I missed something along the way. Google
> Closure was taking a rename map and seemed to be using it.
>
> HTH,
> -Alex
>
> On 3/26/20, 1:23 AM, "Harbs" <[email protected]> wrote:
>
> Sorry to hear. :-(
>
> I still wonder if maybe we can document all the cases where we’re
> relying on not renaming and find a way to deal with those cases. (for
> someone who wants to disable exports)
>
> I know Greg has done a significant amount of digging related to his
> reflection work. Alex, do you have any thoughts?
>
> Harbs
>
> > On Mar 25, 2020, at 9:06 PM, Josh Tynjala <[email protected]>
> wrote:
> >
> > (With credit to Chris for the subject name 😏)
> >
> > Some of you may know that over the last 2-3 months I've been looking
> into
> > ways to add more control over how the compiler handles renaming and
> > exporting APIs in the generated JS. Ideally, my work would have
> culminated
> > in a variety of options that would allow release builds to be much
> smaller,
> > if you were willing to sacrifice certain capabilities (things along
> the
> > lines of dynamic access of properties and reflection). Obviously,
> you would
> > have been able to keep things working the same as they do today, so
> my
> > changes would have been opt-in.
> >
> > I've determined that Closure does not actually expose the ability to
> > prevent renaming of JS APIs, except by exporting them. The advanced
> > compiler APIs that Closure exposes to prevent renaming are
> specifically
> > intended for compiling multiple related projects together (like
> we're doing
> > with modules). Additionally, I discovered that Closure does not
> guarantee
> > that any custom rename mappings will be respected. In fact, the
> > documentation says they may be completely ignored, and there is
> official
> > guidance that says not to do what I was originally trying to do. I
> could
> > not find any other Closure compiler APIs accessible in Java that
> could do
> > what we want here, so I think that our original assumption that
> preventing
> > renaming programmatically would be possible was incorrect.
> >
> > In the case of disabling exports, I found that there are parts of the
> > Royale framework that rely on the fact that properties are exported.
> > Bindings and setting properties in MXML are the most visible. There
> are
> > probably more that I didn't discover yet because the first two had
> such a
> > big impact. Honestly, I am not experienced enough in the framework
> side of
> > Royale to know that I won't break anything if I try to start making
> changes
> > to ensure that all framework code can handle property renaming. Maybe
> > disabling exports is still possible, but I don't think that I'm the
> one who
> > can do it.
> >
> > In the end, I was left feeling extremely frustrated with Closure
> compiler
> > once again. It's not the first time, and it's unlikely to be the
> last. I
> > wasted two whole months trying to follow its incredibly strict rule
> system.
> > In the past, I have tried to fight Closure instead, so I thought
> that I was
> > doing things right this time. I really wish I would have spent that
> time
> > doing something else that would have benefitted Royale more.
> >
> > Side note: As I was reverting the rename/export changes I had made
> to the
> > compiler, I realized that there were some issues with my previous
> attempts
> > to improve the situation with warn-public-vars. With that in mind, I
> > reverted those changes too (for now). I plan to look into a better
> solution
> > for warn-public-vars soon, but I can't guarantee that anything will
> work
> > because it's another place where Closure compiler is the cause of
> the issue.
> >
> > --
> > Josh Tynjala
> > Bowler Hat LLC <
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&data=02%7C01%7Caharui%40adobe.com%7C1967733ab67845f13d6008d7d15ee5b1%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637208077819326597&sdata=X7y97FA75OFUh9hqxLiO5q7W15DoHgCacSRi9G0wh7M%3D&reserved=0
> >
>
>
>
>
