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 something like 
that for modules.

It is one thing to write code that Closure Compiler can minify, it is another 
to make our code harder to work with because we are afraid to understand an 
open source compiler.

Fix the problem in the right place, please.  Please also take the time to 
understand how the compiler works before guessing at solutions.

Thanks,
-Alex

On 1/16/20, 8:53 AM, "Josh Tynjala" <[email protected]> wrote:

    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, and we need to write our code in the style that it demands. Right
    now, we have a bug because we weren't careful about that.
    
    @Greg: Yes, I also thought of a single function could set all properties at
    once. It's certainly an alternative that is worth considering, and I think
    it would nicely avoid any overhead that Alex is worried about.
    @Harbs/@Greg: I'll check out goog.reflect.objectProperty(). If that works,
    we might be able to avoid the function call(s) completely.
    
    --
    Josh Tynjala
    Bowler Hat LLC 
<https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbowlerhat.dev&amp;data=02%7C01%7Caharui%40adobe.com%7C31b3966338eb41f142fc08d79aa4a19f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637147904190228714&amp;sdata=tFTMpbJnM6k27dJWRQQj7mKIcNCLbxcopVzfL5bTILg%3D&amp;reserved=0>
    
    
    On Wed, Jan 15, 2020 at 3:22 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&amp;data=02%7C01%7Caharui%40adobe.com%7C31b3966338eb41f142fc08d79aa4a19f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637147904190228714&amp;sdata=WnWxyfaeoLZAGtGD%2BxRmw9XtyxZrA8PRozvCTle%2B018%3D&amp;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&amp;data=02%7C01%7Caharui%40adobe.com%7C31b3966338eb41f142fc08d79aa4a19f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C637147904190228714&amp;sdata=tFTMpbJnM6k27dJWRQQj7mKIcNCLbxcopVzfL5bTILg%3D&amp;reserved=0>
    >
    

Reply via email to