apache / royale-compiler

Apache Royale Compiler
https://royale.apache.org/
Apache License 2.0
98 stars 50 forks source link

class paths remain in minified output #202

Open Harbs opened 2 years ago

Harbs commented 2 years ago

Using this simple app:

<?xml version="1.0" encoding="utf-8"?>
<js:Application xmlns:fx="http://ns.adobe.com/mxml/2009"
                xmlns:js="library://ns.apache.org/royale/basic"
                applicationComplete="onComplete()">
    <fx:Script>
        <![CDATA[
            private function onComplete():void{
                trace("foo");
            }
        ]]>
    </fx:Script>
    <js:valuesImpl>
        <js:SimpleCSSValuesImpl />
    </js:valuesImpl>
    <js:initialView>
        <js:View>
            <js:Label text="ID" id="id1" />
        </js:View>
    </js:initialView>
</js:Application>

If you compile using this config:

{
    "config": "royale",
    "compilerOptions": {
        "debug": true,
        "targets": ["JSRoyale"],
        "source-map": true,
        "remove-circulars": true
    },
    "additionalOptions": "-js-output-optimization=skipAsCoercions -allow-abstract-classes=true -js-vector-emulation-class=Array -js-vector-index-checks=false -js-complex-implicit-coercions=false -export-public-symbols=false -prevent-rename-protected-symbols=false -prevent-rename-public-static-methods=false -prevent-rename-public-instance-methods=false -prevent-rename-public-static-variables=false -prevent-rename-public-instance-variables=false -prevent-rename-public-instance-accessors=false",
    "files":
    [
        "src/test_project.mxml"
    ]
}

You get this at the top of the minified code:

var aa='Error #1034: Type Coercion failed: cannot convert ',ba='applicationComplete',ca='array',da='backgroundImage',ea='border-box',fa='childrenAdded',f='class',ha='deferredSizeHandler',ia='explicitHeightChanged',ja='explicitWidthChanged',h='function',ka='handleInitComplete',la='handleSizeChange',ma='heightChanged',na='inherit',oa='initComplete',k='interface',pa='number',qa='object',ra='org.apache.royale.core.Application',sa='org.apache.royale.core.ApplicationBase',ta='org.apache.royale.core.Bead',
ua='org.apache.royale.core.BeadViewBase',va='org.apache.royale.core.DispatcherBead',wa='org.apache.royale.core.ElementWrapper',xa='org.apache.royale.core.GroupBase',ya='org.apache.royale.core.HTMLElementWrapper',za='org.apache.royale.core.IApplicationView',Aa='org.apache.royale.core.IBead',Ba='org.apache.royale.core.IBeadController',Ca='org.apache.royale.core.IBeadLayout',Da='org.apache.royale.core.IBeadModel',Ea='org.apache.royale.core.IBeadView',Fa='org.apache.royale.core.IBorderPaddingMarginValuesImpl',
Ga='org.apache.royale.core.ICSSImpl',Ha='org.apache.royale.core.IChild',Ia='org.apache.royale.core.IContainer',Ja='org.apache.royale.core.IContentViewHost',Ka='org.apache.royale.core.IDocument',La='org.apache.royale.core.IFlexInfo',Ma='org.apache.royale.core.IId',Na='org.apache.royale.core.IInitialViewApplication',Oa='org.apache.royale.core.ILayoutChild',Pa='org.apache.royale.core.ILayoutHost',Qa='org.apache.royale.core.ILayoutParent',Ra='org.apache.royale.core.ILayoutView',Sa='org.apache.royale.core.IMXMLDocument',
Ta='org.apache.royale.core.IMeasurementBead',Ua='org.apache.royale.core.IParent',Va='org.apache.royale.core.IParentIUIBase',Wa='org.apache.royale.core.IPopUpHost',Xa='org.apache.royale.core.IPopUpHostParent',Ya='org.apache.royale.core.IRenderedObject',Za='org.apache.royale.core.IRoyaleElement',$a='org.apache.royale.core.IState',ab='org.apache.royale.core.IStatesImpl',bb='org.apache.royale.core.IStatesObject',cb='org.apache.royale.core.IStrand',db='org.apache.royale.core.IStrandWithModel',eb='org.apache.royale.core.IStrandWithModelView',
fb='org.apache.royale.core.IStyleObject',gb='org.apache.royale.core.IStyleableObject',hb='org.apache.royale.core.IUIBase',ib='org.apache.royale.core.IValuesImpl',jb='org.apache.royale.core.LayoutBase',kb='org.apache.royale.core.SimpleCSSValuesImpl',lb='org.apache.royale.core.UIBase',mb='org.apache.royale.core.ValuesManager',nb='org.apache.royale.core.View',ob='org.apache.royale.core.ViewBase',pb='org.apache.royale.core.WrappedHTMLElement',qb='org.apache.royale.core.layout.EdgeData',rb='org.apache.royale.core.layout.LayoutData',
sb='org.apache.royale.core.layout.MarginData',tb='org.apache.royale.core.styles.BorderStyles',ub='org.apache.royale.events.BrowserEvent',vb='org.apache.royale.events.ElementEvents',wb='org.apache.royale.events.Event',xb='org.apache.royale.events.EventDispatcher',yb='org.apache.royale.events.IBrowserEvent',zb='org.apache.royale.events.IEventDispatcher',Ab='org.apache.royale.events.IRoyaleEvent',Bb='org.apache.royale.events.ValueChangeEvent',Cb='org.apache.royale.events.ValueEvent',Db='org.apache.royale.events.utils.EventUtils',
Eb='org.apache.royale.html.Label',Fb='org.apache.royale.html.beads.GroupView',Gb='org.apache.royale.html.beads.layouts.BasicLayout',Hb='org.apache.royale.utils.CSSUtils',Ib='org.apache.royale.utils.MXMLDataInterpreter',Jb='org.apache.royale.utils.StringPadder',Kb='org.apache.royale.utils.Timer',Lb='percentHeightChanged',Mb='percentWidthChanged',Nb='resizeHandler',Ob='sizeChanged',Pb='string',Qb='test_project',Rb='viewChanged',Sb='widthChanged';function l(){return function(){}}

That's a lot of unnecessary cruft.

My best guess on why that's happening is:

  1. Language.as uses ROYALE_CLASS_INFO to find interfaces.
  2. That makes Closure Compiler assume ROYALE_CLASS_INFO is needed.
  3. All the info from ROYALE_CLASS_INFO including the class name and qname is preserved.

Only the interface information is needed by Language. The name and qname are only needed for reflection.

The solution is probably to separate the class name and qname from the interface dependencies. It seems like they should be two variables.

That should free up name and qname to be removed unless Reflection is used.

Names that make sense would be ROYALE_CLASS_INFO and ROYALE_INTERFACE_INFO. A question I have would be whether pulling out the interface info might cause problems for existing SWC libraries. Probably yes, but requiring rebuilding SWCs is probably reasonable.

greg-dove commented 2 years ago

I can look into this, but probably not until late December or early January - feel free to assign to me unless others want to take it on first. On a related note, does anyone know why there is a 'names' Array instance in the ROYALE_CLASS_INFO - we only ever have one element in these, and it is always accessed as ROYALE_CLASS_INFO.names[0].qName, for example. I assume it could be changed to be a single Object with name, qName , etc. Also there are a few places where this is used directly I think (outside of the Reflection lib). Slightly OT: I have vague notions of other ways to support reflective functionality that might be more selective (for tuning the amount of reflection data in the final build, for example) so I might give that a shot at some point too.

joshtynjala commented 2 years ago

Just some random investigation out of curiosity. I couldn't remember if the fully qualified names have always been compiled into release builds like this, or if it was something that was (perhaps unintentionally) introduced at some point more recently. I went as far back as Royale 0.9.0, and that version also included the qnames in release builds. I doubt it was any different in older FlexJS builds.

Harbs commented 2 years ago

Well, it seems like that was only partially the issue. I created a branch (feature/ROYALE_INTERFACE_INFO) which moves the interface data out into a separate variable and Language uses that. That still didn't fix the issue for me and I see that the problem still exists because of SimpleCSSValuesImpl. SimpleCSSValuesImpl uses ROYALE_CLASS_INFO in getValue. I'm pretty sure it uses that to walk up the super-classes to find defaults for beads and the like.

I confirmed that switching to SimpleValuesImpl makes the class names go away. Of course that's not really a solution because that breaks the vast majority of apps.

I'm not sure what the right solution is. Preserving class names for classes which have CSS lookups seems like it might be necessary. I'm not sure how else we can preserve those lookups. Any thoughts on that? But even if yes, not EVERY class needs those lookups. Most classes will never be accessed by the valuesImpl... Not sure what to do...

Harbs commented 2 years ago

One more thing that we probably need to do to prevent rename issues is to make sure the compiler doesn't add quotes to the property names: ROYALE_CLASS_INFO, ROYALE_INTERFACE_INFO and ROYALE_INTERFACE_INFO.

Harbs commented 2 years ago

I added exceptions here: https://github.com/apache/royale-compiler/commit/6c1bd52f8942bff982b09ffcbc7e33dffeb51dfd

I'd appreciate if someone who's more familiar with the compiler code looks at what I did. I don't know if there's a better way to do it...