Re: MXML and warn-public vars

@joshtynj...@apache.org I had trouble wrapping my head around the polarity of this option as well. Public getter/setters and methods are always being renamed, it is when you don't rename public vars that we try to muck with that renaming. I saw your attempt to fix this morning. I agree with

Re: MXML and warn-public vars

On 2/20/20, 6:50 AM, "Carlos Rovira" 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

Re: MXML and warn-public vars

Sorry, yes, of course it's false. Haven't had my morning tea yet. -- Josh Tynjala Bowler Hat LLC On Thu, Feb 20, 2020 at 7:17 AM Josh Tynjala wrote: > Do you mean -rename-public-vars=true instead of false? False should > preserve the original behavior. > > -- > Josh Tyn

Re: MXML and warn-public vars

Do you mean -rename-public-vars=true instead of false? False should preserve the original behavior. -- Josh Tynjala Bowler Hat LLC On Thu, Feb 20, 2020 at 1:44 AM Alex Harui wrote: > This change is breaking some modules when -rename-public-vars=false. > Let's say the ma

Re: MXML and warn-public vars

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? Thanks El jue., 20 feb. 2020 a las 10:44, Alex Harui

Re: MXML and warn-public vars

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

Re: MXML and warn-public vars

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 On Wed, Feb 5, 2020 at 12:54 PM Greg Dove wrote

Re: MXML and warn-public vars

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: (I have never tried this with uppercase first letter in state names, but presume it wor

Re: MXML and warn-public vars

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 On Wed, Feb 5, 2020 a

Re: MXML and warn-public vars

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

Re: MXML and warn-public vars

Yeah, I'll make sure that users can control whether renaming happens or not. -- Josh Tynjala Bowler Hat LLC On Wed, Feb 5, 2020 at 11:51 AM Alex Harui wrote: > Great work, Josh! > > I have to say that the objectProperty output was adding pain to debugging > so looking f

Re: MXML and warn-public vars

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

Re: MXML and warn-public vars

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 a

Re: MXML and warn-public vars

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 chang

Re: MXML and warn-public vars

Comments inline. On Thursday, January 16, 2020, Alex Harui 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

Re: MXML and warn-public vars

I'm not sure that I understand how this solution takes control away from anyone. It fixes a bug when a public variable is renamed, but at the same time, it preserves the existing behavior for things that are not renamed. - Josh On Thursday, January 16, 2020, Alex Harui wrote: > I'm not surprised

Re: MXML and warn-public vars

I'm not surprised that this change fixed a problem caused by removing @export. This is not a question about the technical accuracy of your work, which is always very good. This just doesn't feel like the right solution. Maybe we should start by agreeing on facts and then goals and then discus

Re: MXML and warn-public vars

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

Re: MXML and warn-public vars

Even if we find a way to prevent renaming, some people are going to want to keep renaming enabled to minimize the size of their generated JS. In that situation, goog.reflect.objectProperty() will still be necessary. With that in mind, it may not be the full solution, but it's one step closer. -- J

Re: MXML and warn-public vars

On 1/16/20, 2:24 PM, "Harbs" wrote: In other words reflection? The reflection classes already deal with this IIUC. You’re right that a simplistic serializer will not work, but for that they wouldn’t be able to take advantage of the advanced minification anyway. They

Re: MXML and warn-public vars

In other words reflection? The reflection classes already deal with this IIUC. You’re right that a simplistic serializer will not work, but for that they wouldn’t be able to take advantage of the advanced minification anyway. The recommended solution should be to use reflection classes or disab

Re: MXML and warn-public vars

On 1/16/20, 2:14 PM, "Harbs" wrote: What do you mean? This is the solution recommended by the compiler. What are your concerns? Someone writes their own deserializer. It is going to do something like: Var listOfProperties = ["firstName", "lastName"]; Function deserialize(inputS

Re: MXML and warn-public vars

What do you mean? This is the solution recommended by the compiler. What are your concerns? > On Jan 17, 2020, at 12:11 AM, Alex Harui wrote: > > OK. I still think this is not the right solution. As I said public vars are > for more than MXML.

Re: MXML and warn-public vars

On 1/16/20, 2:08 PM, "Harbs" wrote: Uh. No. You’d see data = [function, 1, ‘publicVar', true, 'value’] It’s a function call which returns the fed parameter. OK. I still think this is not the right solution. As I said public vars are for more than MXML. -Alex

Re: MXML and warn-public vars

Uh. No. You’d see data = [function, 1, ‘publicVar', true, 'value’] It’s a function call which returns the fed parameter. > On Jan 17, 2020, at 12:06 AM, Alex Harui wrote: > > And so when I have to debug the structure in js-debug, I will see > > data = [function, 1, function, true, 'value'] >

Re: MXML and warn-public vars

And so when I have to debug the structure in js-debug, I will see data = [function, 1, function, true, 'value'] And have no idea what property is being set? I'm not in favor of this change. The public var warning is not just for MXML. It is for any dynamic access by property name whether from

Re: MXML and warn-public vars

Before: var data = [ Component, 1, 'publicVar', true, 'value' ] After (debug): var data = [ Component, 1, goog.reflect.objectProperty('publicVar'), true, 'value' ] In a release build, Closure compiler replaces the function call completely with the minifie

Re: MXML and warn-public vars

Where does the goog.reflect.objectProperty appear in the output? In the data stream? On 1/16/20, 1:35 PM, "Harbs" wrote: Amazing! > On Jan 16, 2020, at 10:59 PM, Josh Tynjala wrote: > > Thank you, Harbs! Wrapping the variable name in a > goog.reflect.objectProper

Re: MXML and warn-public vars

Amazing! > On Jan 16, 2020, at 10:59 PM, Josh Tynjala 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

Re: MXML and warn-public vars

That sounds great, thanks for working on that Josh! I was meaning to take a look into the same goog stuff for possible optimizations in reflection data and AMF, but I don't think any of that is in the same league as the improvement you just made (because they currently work in release build as-is)

Re: MXML and warn-public vars

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 func

Re: MXML and warn-public vars

Simple is usually best. Changing what is currently a simple data stream into something that cannot be easily seen in the debugger is adding complexity. I'm pretty sure we can control the renaming in the Closure Compiler without changing how we write our code for them. We are already doing som

Re: MXML and warn-public vars

I should add one thing that I remembered soon after sending this. If we want to get rid of @export to ultimately remove public APIs that aren't used, a similar pattern will probably be necessary for setters too. Setters without @export get renamed too. Ultimately, we chose to use Closure compiler,

Re: MXML and warn-public vars

I’m not sure that this increases function call overhead or code size. I suspect it will have the opposite effect. Basically, to avoid function calls in MXML structure, we’re requiring getters and setters in every public member. That results in two function calls in every class instantiation per

Re: MXML and warn-public vars

We've been down this road before. I would rather warn folks to change their code so it will be as small and fast as possible. Others don't seem to care and would rather not change their code. The end-of-the-road for the "must mimic Flex" is that we will rarely read and write variables/propert

Re: MXML and warn-public vars

Am I misunderstanding or does this fix a usability issue with mxml and public vars? If it is fixing something that does not work, then presumably it only 'costs' for people who want that thing to work (public var assignments in mxml instances?) and not for others. That seems like the app-dev still

Re: MXML and warn-public vars

Clever idea, but I don't like it. We are trying to reduce function call overhead and reduce download size, not increase it. The reasons you are getting a public var warning is so that the app-dev has control over which members will have function call overhead. I don't think we should take that

Re: MXML and warn-public vars

FWIW, all the functions in goog.reflect are worthy of a look. > On Jan 16, 2020, at 6:32 AM, Harbs wrote: > > Sounds good! > > https://github.com/google/closure-compiler/wiki/Type-Based-Property-Renaming > > > The

Re: MXML and warn-public vars

Sounds good! https://github.com/google/closure-compiler/wiki/Type-Based-Property-Renaming The function seems to be goog.reflect.objectProperty() I’m not sure exactly how it works though. > On Jan 16, 2020, at 1:37 A

Re: MXML and warn-public vars

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

Re: MXML and warn-public vars

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 assignm

MXML and warn-public vars

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://github.com/apache/royale-compiler/commit/eed5882ba935870a98ba4fe8cbf499e5d8344f60 In other words, this MXML code