Thanks for this Josh!

I just tested this on a very complex library project with crazy complex untyped 
data structures. It works great!

I wish we had this option when I was working on my app. I spent many hours 
chasing down minification issues that could have been fixed by this option.

I also think it should default to true.

Thanks,
Harbs

> On Jun 26, 2018, at 9:56 PM, [email protected] wrote:
> 
> This is an automated email from the ASF dual-hosted git repository.
> 
> joshtynjala pushed a commit to branch develop
> in repository https://gitbox.apache.org/repos/asf/royale-compiler.git
> 
> 
> The following commit(s) were added to refs/heads/develop by this push:
>     new 48d86c8  compiler-jx: added js-dynamic-access-unknown-members 
> compiler option to have emitter use dynamic bracket access when the right 
> side of a member access expression cannot be resolved to a definition.
> 48d86c8 is described below
> 
> commit 48d86c8c4b235ce60b282ad16d62d4ff87c3746b
> Author: Josh Tynjala <[email protected]>
> AuthorDate: Tue Jun 26 11:55:56 2018 -0700
> 
>    compiler-jx: added js-dynamic-access-unknown-members compiler option to 
> have emitter use dynamic bracket access when the right side of a member 
> access expression cannot be resolved to a definition.
> 
>    In other words, myObj.dynamicMember will become myObj['dynamicMember']. 
> Known members will use the regular dot syntax. Useful for stopping the 
> Closure compiler from renaming members that are dynamic, such as members of 
> an object literal. Defaults to false to avoid breaking anything. However, I 
> think it makes sense to have it default to true instead to give developers a 
> better user experience.
> ---
> .../royale/compiler/clients/JSConfiguration.java   | 26 ++++++++
> .../codegen/js/jx/MemberAccessEmitter.java         | 47 +++++++++++---
> .../js/royale/TestDynamicAccessUnknownMembers.java | 71 ++++++++++++++++++++++
> 3 files changed, 137 insertions(+), 7 deletions(-)
> 
> diff --git 
> a/compiler-jx/src/main/java/org/apache/royale/compiler/clients/JSConfiguration.java
>  
> b/compiler-jx/src/main/java/org/apache/royale/compiler/clients/JSConfiguration.java
> index 0e6caf5..5433608 100644
> --- 
> a/compiler-jx/src/main/java/org/apache/royale/compiler/clients/JSConfiguration.java
> +++ 
> b/compiler-jx/src/main/java/org/apache/royale/compiler/clients/JSConfiguration.java
> @@ -137,6 +137,32 @@ public class JSConfiguration extends Configuration
>     }
> 
>     //
> +    // 'js-dynamic-access-unknown-members'
> +    //
> +
> +    private boolean jsDynamicAccessUnknownMembers = false;
> +
> +    public boolean getJsDynamicAccessUnknownMembers()
> +    {
> +        return jsDynamicAccessUnknownMembers;
> +    }
> +
> +    /**
> +     * If the definition of a member cannot be resolved, emit dynamic access
> +     * instead of normal member access. Ensures that dynamic members aren't
> +     * renamed.
> +     * 
> +     * <code>myObject.memberAccess</code> becomes 
> <code>myObject["memberAccess"]</code>
> +     */
> +    @Config
> +    @Mapping("js-dynamic-access-unknown-members")
> +    public void setJsDynamicAccessUnknownMembers(ConfigurationValue cv, 
> boolean value)
> +            throws ConfigurationException
> +    {
> +        jsDynamicAccessUnknownMembers = value;
> +    }
> +
> +    //
>     // 'compiler.js-external-library-path' option
>     //
> 
> diff --git 
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/MemberAccessEmitter.java
>  
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/MemberAccessEmitter.java
> index d308d34..b7a577c 100644
> --- 
> a/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/MemberAccessEmitter.java
> +++ 
> b/compiler-jx/src/main/java/org/apache/royale/compiler/internal/codegen/js/jx/MemberAccessEmitter.java
> @@ -32,12 +32,14 @@ import 
> org.apache.royale.compiler.internal.codegen.js.goog.JSGoogEmitterTokens;
> import 
> org.apache.royale.compiler.internal.codegen.js.jx.BinaryOperatorEmitter.DatePropertiesGetters;
> import org.apache.royale.compiler.internal.definitions.AccessorDefinition;
> import org.apache.royale.compiler.internal.definitions.FunctionDefinition;
> +import org.apache.royale.compiler.internal.projects.RoyaleJSProject;
> import org.apache.royale.compiler.internal.tree.as.DynamicAccessNode;
> import org.apache.royale.compiler.internal.tree.as.FunctionCallNode;
> import org.apache.royale.compiler.internal.tree.as.GetterNode;
> import org.apache.royale.compiler.internal.tree.as.IdentifierNode;
> import org.apache.royale.compiler.internal.tree.as.MemberAccessExpressionNode;
> import 
> org.apache.royale.compiler.internal.tree.as.NamespaceAccessExpressionNode;
> +import org.apache.royale.compiler.projects.ICompilerProject;
> import org.apache.royale.compiler.tree.ASTNodeID;
> import org.apache.royale.compiler.tree.as.IASNode;
> import org.apache.royale.compiler.tree.as.IExpressionNode;
> @@ -255,17 +257,48 @@ public class MemberAccessEmitter extends JSSubEmitter 
> implements
>                       getEmitter().emitClosureStart();
>               
>               continueWalk = writeLeftSide(node, leftNode, rightNode);
> -            if (continueWalk && !isCustomNamespace)
> -            {
> -                startMapping(node, node.getLeftOperandNode());
> -                write(node.getOperator().getOperatorText());
> -                endMapping(node);
> -            }
>         }
> 
>         if (continueWalk)
>         {
> -            getWalker().walk(node.getRightOperandNode());
> +                     boolean emitDynamicAccess = false;
> +            boolean dynamicAccessUnknownMembers = false;
> +            ICompilerProject project = getProject();
> +            if(project instanceof RoyaleJSProject)
> +            {
> +                RoyaleJSProject fjsProject = (RoyaleJSProject) project;
> +                if(fjsProject.config != null)
> +                {
> +                    dynamicAccessUnknownMembers = 
> fjsProject.config.getJsDynamicAccessUnknownMembers();
> +                }
> +            }
> +                     if (dynamicAccessUnknownMembers && rightNode instanceof 
> IIdentifierNode)
> +                     {
> +                             IIdentifierNode identifierNode = 
> (IIdentifierNode) node.getRightOperandNode();
> +                             IDefinition resolvedDefinition = 
> identifierNode.resolve(getProject());
> +                             emitDynamicAccess = resolvedDefinition == null;
> +                     }
> +                     if (emitDynamicAccess)
> +                     {
> +                             IIdentifierNode identifierNode = 
> (IIdentifierNode) node.getRightOperandNode();
> +                             startMapping(node, rightNode);
> +                             write(ASEmitterTokens.SQUARE_OPEN);
> +                             write(ASEmitterTokens.DOUBLE_QUOTE);
> +                             write(identifierNode.getName());
> +                             write(ASEmitterTokens.DOUBLE_QUOTE);
> +                             write(ASEmitterTokens.SQUARE_CLOSE);
> +                             endMapping(node);
> +                     }
> +                     else
> +                     {
> +                             if (!isStatic && !isCustomNamespace)
> +                             {
> +                                     startMapping(node, 
> node.getLeftOperandNode());
> +                                     
> write(node.getOperator().getOperatorText());
> +                                     endMapping(node);
> +                             }
> +                             getWalker().walk(node.getRightOperandNode());
> +                     }
>         }
> 
>         if (needClosure)
> diff --git 
> a/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestDynamicAccessUnknownMembers.java
>  
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestDynamicAccessUnknownMembers.java
> new file mode 100644
> index 0000000..64e5413
> --- /dev/null
> +++ 
> b/compiler-jx/src/test/java/org/apache/royale/compiler/internal/codegen/js/royale/TestDynamicAccessUnknownMembers.java
> @@ -0,0 +1,71 @@
> +/*
> + *
> + *  Licensed to the Apache Software Foundation (ASF) under one or more
> + *  contributor license agreements.  See the NOTICE file distributed with
> + *  this work for additional information regarding copyright ownership.
> + *  The ASF licenses this file to You under the Apache License, Version 2.0
> + *  (the "License"); you may not use this file except in compliance with
> + *  the License.  You may obtain a copy of the License at
> + *
> + *      http://www.apache.org/licenses/LICENSE-2.0
> + *
> + *  Unless required by applicable law or agreed to in writing, software
> + *  distributed under the License is distributed on an "AS IS" BASIS,
> + *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
> + *  See the License for the specific language governing permissions and
> + *  limitations under the License.
> + *
> + */
> +
> +package org.apache.royale.compiler.internal.codegen.js.royale;
> +
> +import org.apache.royale.compiler.driver.IBackend;
> +import org.apache.royale.compiler.exceptions.ConfigurationException;
> +import 
> org.apache.royale.compiler.internal.driver.js.goog.JSGoogConfiguration;
> +import org.apache.royale.compiler.internal.driver.js.royale.RoyaleBackend;
> +import org.apache.royale.compiler.internal.projects.RoyaleJSProject;
> +import org.apache.royale.compiler.internal.test.ASTestBase;
> +import org.apache.royale.compiler.tree.as.IMemberAccessExpressionNode;
> +import org.junit.Test;
> +
> +public class TestDynamicAccessUnknownMembers extends ASTestBase
> +{
> +    @Override
> +    public void setUp()
> +    {
> +        backend = createBackend();
> +        project = new RoyaleJSProject(workspace, backend);
> +     JSGoogConfiguration config = new JSGoogConfiguration();
> +     try {
> +                     config.setJsDynamicAccessUnknownMembers(null, true);
> +             } catch (ConfigurationException e) {
> +                     // TODO Auto-generated catch block
> +                     e.printStackTrace();
> +             }
> +     project.config = config;
> +        super.setUp();
> +     }
> +
> +    protected IBackend createBackend()
> +    {
> +        return new RoyaleBackend();
> +    }
> +
> +    @Test
> +     public void testVisitKnownMember()
> +    {
> +        IMemberAccessExpressionNode node = (IMemberAccessExpressionNode) 
> getNode(
> +                "public class KnownMember { public function KnownMember() { 
> this.knownMember = 4; } public var knownMember:Number; }", 
> IMemberAccessExpressionNode.class, WRAP_LEVEL_PACKAGE);
> +        asBlockWalker.visitMemberAccessExpression(node);
> +        assertOut("this.knownMember");
> +    }
> +
> +    @Test
> +     public void testVisitUnknownMember()
> +    {
> +        IMemberAccessExpressionNode node = (IMemberAccessExpressionNode) 
> getNode(
> +                "public dynamic class KnownMember { public function 
> UnknownMember() { this.unknownMember = 4; } public var knownMember:Number; 
> }", IMemberAccessExpressionNode.class, WRAP_LEVEL_PACKAGE);
> +        asBlockWalker.visitMemberAccessExpression(node);
> +        assertOut("this[\"unknownMember\"]");
> +    }
> +}
> \ No newline at end of file
> 

Reply via email to