flex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Dove <greg.d...@gmail.com>
Subject Re: [FlexJX][Falcon] Binding support fixes/improvements
Date Sat, 03 Sep 2016 01:41:57 GMT
Sound great. Yes, switching it back on at any point should remain a quick
fix option because it would simply make the other code act as a backstop.
I will take a quick look on Monday to see if I can find any problems in
small as-only projects where maybe the order of class definitions in the
swf could be important with the current approach.  Sounds a bit like a
theoretical edge case but you made me think about it.
Have a nice weekend.

-Greg
[sent from my phone]

On 3/09/2016 12:41 PM, "Alex Harui" <aharui@adobe.com> wrote:

>
>
> On 9/2/16, 5:10 PM, "Greg Dove" <greg.dove@gmail.com> wrote:
>
> >Yes, that was one of the major changes. I moved it alongside the other
> >implementation inside ClassDirectiveProcessor and was able to get more
> >consistent results.
> >I initially commented it out so I could get everything working using the
> >implements IEventDispatcher approach - which is the more general approach
> >that would work for all cases- in javascript, then added it back for both
> >targets as a 2nd output variation in a later commit, and it went to
> >ClassDirectorProcessor for swf.
> >
> >I was seeing 2 issues from the ASCompilationUnit implementation:
> >1. It was not always applying where it should have been (so in cases where
> >it could have been 'extends EventDispatcher' it would sometimes fall
> >through to the implements IEventDispatcher approach in
> >ClassDIrectiveProcessor anyway). I suspect this was threading-related, but
> >I am not so familiar with threads.
> >
> >2. Less of an issue, it could also apply an EventDispatcher base class in
> >static-only bindable cases, which is unnecessary, this I expect could have
> >been more easily fixed. I tried checking with hasModifier in the original
> >code, but I think that might not be available yet, iirc that caused an
> >error.
> >
> >So I moved everything to the ClassDirectiveProcessor, which had the other
> >implementation of bindable support in any case. Sorry, I thought I had
> >been
> >clear about that.
>
> Well, you probably did explain it, but it probably didn't stick in my head
> until we hit this last issue and I went looking for the code where I had a
> vague memory of hacking a base class.  I still don't know the compiler
> code so well that it is clear in my head what work should be done where.
> It looks like the original attempt to deal with [Bindable] for SWF was
> done in ClassDirectiveProcessor, but when I wanted to fix a bug in
> FalconJX I decided to fix it by hacking the AST since that drives
> everything in FalconJX.  The right answer may have been for me to move the
> code from ClassDirectiveProcessor into ASCompilationUnit and use the
> existing tests for needsEventDispatcher, then maybe we wouldn't have had
> the two issues above.
>
> Anyway, I think we have recorded the possibility that AST hacking might be
> a solution if we hit other problems.  I'm ok with leaving the code as is
> until we hit another problem but feel free to keep digging if you want.
>
> Thanks,
> -Alex
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message