I'm not sure I understand your proposal.  Are you proposing that all public 
variables will be quoted and thus not renamed so we never warn again?  I am not 
in favor of that.  IMO, the vast majority of public vars can be renamed without 
penalty.  It is only certain classes that will be mapped to external code that 
matter.

My 2 cents,
-Alex

On 9/16/19, 10:46 AM, "Josh Tynjala" <[email protected]> wrote:

    You're right. After a number of tests, I cannot find any annotation (or
    combination of them) that will prevent the renaming of variables defined on
    a prototype. All of the official advice that I see from Google suggests
    quoting the properties (or using externs). So, from a Royale perspective,
    it looks like quoting the public variable's name is our best option.
    
    Similar to how we handle js-dynamic-access-unknown-members, we can quote
    the public variable's name when getting or setting it in JS:
    
    var foo = new Foo();
    foo["bar"] = 2;
    var baz = foo["bar"];
    
    In this case, we also need to quote the public variable's name when
    declaring it on the prototype:
    
    Foo.prototype["bar"] = 3;
    
    I did some tests, and I can confirm that Closure will preserve the public
    variable's name, similar to how it preserves the names when we declare
    public getters and setters. I'm going to make this change and turn off
    warn-public-vars.
    
    (To be clear, I'm only going to change how the emitter handles public
    variables specifically. Behavior for other types of property access will
    remain the same.)
    
    --
    Josh Tynjala
    Bowler Hat LLC 
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&amp;data=02%7C01%7Caharui%40adobe.com%7Cbc8634d75ac34b1ff2a408d73acdc1c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637042527741705917&amp;sdata=oIgajcsJSzyrmkeIOX%2BcuPe9lVt7iukyGAhrLdlVmvY%3D&amp;reserved=0>
    
    
    On Mon, Sep 16, 2019 at 10:06 AM Alex Harui <[email protected]>
    wrote:
    
    > Feel free to revisit.  IIRC, the issue is that you can't @export a "var".
    > You can only @export a function because @export sets up a reference to the
    > renamed function and you can't set up a reference to simple vars.
    >
    > Class Josh {
    >   Static var foo:int = 1;
    > }
    >
    > Compiles to:
    >
    >   Josh.foo = 1;
    >
    > Gets renamed to:
    >
    >   a.b = 1;
    >
    > The @export will result in:
    >
    >   ['Josh']['foo'] = a.b;
    >
    > And then code that does:
    >
    >   Josh.foo = 2;
    >
    > Will not change
    >
    >   Console.log(a.b); // 1 (not 2)
    >
    > Of course, I could be wrong..
    >
    > -Alex
    >
    > On 9/16/19, 9:57 AM, "Josh Tynjala" <[email protected]> wrote:
    >
    >     I guess I was assuming that @nocollapse combined with @export would
    > make it
    >     also prevent renaming. I suppose I can test it and see what happens.
    >
    >     --
    >     Josh Tynjala
    >     Bowler Hat LLC <
    > 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&amp;data=02%7C01%7Caharui%40adobe.com%7Cbc8634d75ac34b1ff2a408d73acdc1c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637042527741715916&amp;sdata=RzTt0oXl8VlOFQx61iqQmpTnZjzRQ83WA0nhC14VaBU%3D&amp;reserved=0
    > >
    >
    >
    >     On Mon, Sep 16, 2019 at 9:54 AM Alex Harui <[email protected]>
    > wrote:
    >
    >     > IMO, the -warn-public-vars is more about the "renaming" mentioned in
    > that
    >     > link than the "collapse".
    >     >
    >     > If you have:
    >     >
    >     > Package {
    >     > Class Josh {
    >     >   Public var name:String;
    >     > }
    >     > Var foo:Josh = new Josh();
    >     > foo.name = 'josh';
    >     >
    >     > I don't think @nocollapse will prevent renaming the 'name' property
    > to
    >     > something random like 'jt':
    >     >
    >     > Var foo = new Josh();
    >     > foo.jt = 'josh';
    >     >
    >     > AIUI, @nocollapse is used for things that are not obviously
    > mutable.  If
    >     > you look in the globals in the browser debugger for any js-debug
    > version of
    >     > a Royale app, you can see the structure:
    >     >
    >     >    org.apache.royale.core
    >     >
    >     > It was created by code similar to:
    >     >
    >     > window.org = {};
    >     > window.org.apache = {};
    >     > window.org.apache.royale = {};
    >     > window.org.apache.royale.core = {};
    >     >
    >     > Then some class gets added:
    >     >
    >     > window.org.apache.royale.core.UIBase = function ()...
    >     >
    >     > And some static might get added to that:
    >     >
    >     > window.org.apache.royale.core.UIBase.FOO = "BAR";
    >     >
    >     > Closure will collapse org.apache.royale.core to just something like
    > "bb"
    >     > in the js-release output.  And that means that code that sets
    > UIBase.FOO
    >     > will break because there won't be an org.apache.royale.core
    > structure.
    >     > AIUI, nocollapse would prevent collapsing that structure, but won't
    > prevent
    >     > UIBase and FOO from being renamed.  And the renaming mainly causes
    > problems
    >     > when deserializing data structures coming from a server or other
    > external
    >     > source.
    >     >
    >     > Of course, I could be wrong...
    >     > -Alex
    >     >
    >     > On 9/16/19, 9:07 AM, "Josh Tynjala" <[email protected]>
    > wrote:
    >     >
    >     >     I was looking through the Closure compiler annotations, and I
    > noticed
    >     >     @nocollapse:
    >     >
    >     >     Denotes a property that should not be collapsed by the compiler
    > into a
    >     >     > variable. The primary use for @nocollapse is to allow
    > exporting of
    >     > mutable
    >     >     > properties.
    >     >     >
    >     >
    >     >
    >     >
    > 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fgoogle%2Fclosure-compiler%2Fwiki%2FAnnotating-JavaScript-for-the-Closure-Compiler%23nocollapse&amp;data=02%7C01%7Caharui%40adobe.com%7Cbc8634d75ac34b1ff2a408d73acdc1c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637042527741715916&amp;sdata=ec7%2Fx1T5lrVGtX8%2BhsScOgeJ18JTrfe8lbk0rm%2BrqUE%3D&amp;reserved=0
    >     >
    >     >     Isn't this collapsing behavior the reason why we needed to add
    >     >     -warn-public-vars?
    >     >
    >     >     --
    >     >     Josh Tynjala
    >     >     Bowler Hat LLC <
    >     >
    > 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&amp;data=02%7C01%7Caharui%40adobe.com%7Cbc8634d75ac34b1ff2a408d73acdc1c7%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637042527741715916&amp;sdata=RzTt0oXl8VlOFQx61iqQmpTnZjzRQ83WA0nhC14VaBU%3D&amp;reserved=0
    >     > >
    >     >
    >     >
    >     >
    >
    >
    >
    

Reply via email to