royale-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Dove <greg.d...@gmail.com>
Subject Re: Revisiting the old debate: 'localId vs. id'
Date Fri, 02 Nov 2018 18:49:46 GMT
Alex, sorry if I wasn't clear.

What I meant was this:
1) The compiler has always had a notion of IDs and effectiveIDs.  IDs
reflect the "id" property in an MXML Instance.  You set id="foo" and the
compiler will create a getter/setter with bindable events named "foo" on
the output class.

I was just matching the above id that for localId in js with the belief
that they should work the same locally without any other changes (in
actionscript/mxml). My changes should only do that for the localId, not all
effectiveIds (if not that was not my intention and I will fix it).
I assumed this is what was happening in swf because I can see the bindings
working in my side-by-side comparison tests, but it could be because of
timing of binding initialization maybe. I will check this.
I had the impression that id and localId were supposed to be functionally
equivalent, with only the HtmlElement setting not happening in js. Maybe
the

I will think about how to optimize things.





On Sat, Nov 3, 2018 at 7:20 AM Alex Harui <aharui@adobe.com.invalid> wrote:

> Hi Greg,
>
> I’m not sure what you mean by "match the swf behavior".  I don't think the
> SWF output generated bindable getter/setters for every effectiveID, but it
> might already have the smarts to do that for any effectiveID it finds is
> used in a source expression for databinding.  In fact, I guess I'm
> surprised that the warning I thought was being generated went away if you
> only changed the JS output.  I thought that warning came from a check
> elsewhere in code that dictates both SWF and JS compile errors.
>
> Getters/setters have function call overhead compared to a plain var, so
> any time we can skip using them, we have faster smaller code.  So Ideally,
> the compiler would only generate getter/setters for "id" when it absolutely
> has to.  So if you can, it would be best to try to make that change a bit
> smarter.  There is a BindingDataBase that might contain useful information.
>
> I agree that you can assume multiple instances for an MXML file in a SWC,
> but I'm not clear that everyone generates SWCs for their MXML files.
>
> Anyway, if we can agree that we could essentially treat "id" like we are
> currently treating "localId" for an entire file, then we don't need a
> "localId" compile-time property which would make Royale more compatible
> with existing IDEs.
>
> My 2 cents,
> -Alex
>
> On 11/2/18, 10:58 AM, "Greg Dove" <greg.dove@gmail.com> wrote:
>
>     Hi Alex, Thanks for the comprehensive info!
>
>     Just a few selective comments:
>
>     'Greg's changes appear to generate bindable getter/setters for all
>     localIDs.  This will work for now, but IMO, isn't as PAYG as it could
> be.'
>
>     Sorry I was not clear in my understanding if this was intentionally
>     omitted. I thought that localId was the same as id, but just avoids the
>     HTMLElement id setting. So I expected switching id to localId to
> continue
>     to work the same but fix the browser console duplicate id alerts. That
> is
>     what I was addressing here. But I think my changes in js match the swf
>     behavior now?
>
>     I'm not proposing anything added to UIBase, just a different way to
>     implement the compile-time feature.
>
>     'One question I have is whether the developer of an MXML Component
> "knows"
>     that the component  is intended to have multiple instances or not.  If
> not,
>     the problem gets harder, as the generated output need to be told
> whether to
>     set the HTMLElement id or not.  If the developer "knows", we could find
>     some way to tell the compiler to generate all "Id" as what we are
> currently
>     generating for "localId".  '
>
>     If the MXML Component is part of a swc, it is safest to assume that it
> is
>     possible to have multiple instances I think. But not really known
> (although
>     some component types can be assumed to be likely).
>
>     'Thinking about it now, I can't think of why, in a single MXML file,
> you
>     would need to sometimes set "id" and other times set "localId".   I
> think
>     either you want all ids in a file to not set the HTMLElement ids or
> not. '
>
>     This seems true also, thinking about it - that's good insight.
>
>
>
>     On Sat, Nov 3, 2018 at 6:25 AM Alex Harui <aharui@adobe.com.invalid>
> wrote:
>
>     > Greg's suggestion is valid.  And there could certainly be a better
>     > solution than "localID".  But maybe we need agreement on the problem
> space
>     > first.
>     >
>     > IMO, the problem of multiple IDs is rare.  Can we agree on that?  My
> guess
>     > is that 90% of .MXML files never have more than one instance of them
> on
>     > screen at a time.
>     >
>     > So, if we can agree on that, then we want to apply PAYG to the
> solution.
>     > We want folks to be able to create an MXML file for the 90% case,
> use IDs,
>     > use CSS and third-party libraries that call getElementById and
> things "just
>     > work".  Hopefully, we can agree that it is ok for more work to be
> required
>     > by the developer and more code to be generated and run to have
> multiple
>     > instances of an MXML file on screen.
>     >
>     > Technically there are two sections of code that factor into this:
>     >
>     > 1) The compiler has always had a notion of IDs and effectiveIDs.  IDs
>     > reflect the "id" property in an MXML Instance.  You set id="foo" and
> the
>     > compiler will create a getter/setter with bindable events named
> "foo" on
>     > the output class.  This is super important in Flash since classes are
>     > sealed (not dynamic) and so you must declare slots to hold
> references to
>     > instances on the output class. But there are cases where an instance
> is
>     > referenced by some other piece of the MXML file but the developer
> did not
>     > specify an id.  Binding expressions can do that.  I think there are
> other
>     > scenarios, but I can't think of them right now.  In these cases the
>     > compiler generates an effectiveID and a simple private var on the
> output
>     > class for every effectiveID.  In the MXML output, the effectiveID is
> given
>     > the property name "_id".
>     >
>     > 2)  The framework code has UIBase with an "id" property setter that
> sets
>     > the id on the HTMLElement.  In addition, the MXMLDataIntepreter has
> logic
>     > that tests if a property being set on an instance is named "id" or
> "_id".
>     > If "id", additional logic sets the slot on the document and sets the
> "id"
>     > property on the instance to set the HTMLElement's id.  If "_id", it
> only
>     > sets the slot on the document, but not the instance, since it could
> assume
>     > that no other code should care that the instance has its id set (and
> thus,
>     > for browser versions, whether the HTMLElement id is set).
>     >
>     > "localId" is a "compile-time property".  It is one of a few
> properties
>     > that don't actually exist on the instance's ActionScript
> implementation.
>     > Other examples are "includeIn" and "excludeFrom" for states.  So, the
>     > localId" doesn't introduce a new problem for IDEs, they all had to
> deal
>     > with includeIn/excludeFrom already, but it is true that IDEs still
> need to
>     > learn about it.
>     >
>     > The localID implementation before Greg's changes leveraged the
> effectiveID
>     > aspect of all of this code.  It did not generate bindable
> getter/setters
>     > "just in case" the element with the localID was used in Bindings.
> It did
>     > not set the instance's id which would run code to set the
> HTMLElement's
>     > id.  However, if the element with a localId was used in a binding
>     > expression then you would get a warning.
>     >
>     > Greg's changes appear to generate bindable getter/setters for all
>     > localIDs.  This will work for now, but IMO, isn't as PAYG as it
> could be.
>     > Ideally, the compiler would find out if the localID is used in the
> source
>     > expression of a binding expression and only then generate the
> bindable
>     > getter/setter.  FWIW, another possible fix would be to suppress the
>     > warning, but there might be a timing issue around effectveIDs used in
>     > states.
>     >
>     > IMO, any proposal to add another actual property on every instance of
>     > UIBase "just-in-case" someone is going to use multiple instances of
> an MXML
>     > file doesn't seem PAYG to me.  This is why we originally chose the
> current
>     > implementation.  Any proposal that makes the setter for "id" do an
> extra
>     > check "just-in-case" the instance is used in multiple instances of
> an MXML
>     > file doesn't seem PAYG to me either.  We could change the meaning
> of, or
>     > name of "localID", but then it is still a compile-time property that
> IDE's
>     > have to handle.
>     >
>     > One question I have is whether the developer of an MXML Component
> "knows"
>     > that the component  is intended to have multiple instances or not.
> If not,
>     > the problem gets harder, as the generated output need to be told
> whether to
>     > set the HTMLElement id or not.  If the developer "knows", we could
> find
>     > some way to tell the compiler to generate all "Id" as what we are
> currently
>     > generating for "localId".   Thinking about it now, I can't think of
> why, in
>     > a single MXML file, you would need to sometimes set "id" and other
> times
>     > set "localId".   I think either you want all ids in a file to not
> set the
>     > HTMLElement ids or not.
>     >
>     > And if that's true, then we can think of ideas to treat the id
> property in
>     > a file instead of a per-MXML-tag way.   One way to do that is a
> compiler
>     > option, but then you would have to compile that MXML file separately
> (or
>     > with other multi-instance MXML files).  Another is some sort of
>     > "directive", maybe metadata or a special comment.   In AS files, we
> already
>     > have special metadata and comments for compiler directives.
>     >
>     > Of course, I could be wrong...
>     > -Alex
>     >
>     > On 11/2/18, 10:13 AM, "Greg Dove" <greg.dove@gmail.com> wrote:
>     >
>     >     Hi Piotr,
>     >
>     >     Thanks for your interest in this. Just to be clear, I don't want
> to
>     > claim
>     >     that this is 'my idea' - it's more a re-visit of things that
> have been
>     >     discussed before, and is probably very similar to some options
> that
>     > were
>     >     decided against previously. I just wondered if anyone had
> changed their
>     >     mind about this. I'm only raising it after some initial use of
> localId
>     > and
>     >     just my using reaction to that experience (possibly heavily
> influenced
>     > by
>     >     the red messages I see in Intellij).
>     >
>     >     At the moment we have
>     >
>     >     <instance id="setDOMid" />
>     >     <instance localId="doNotSetDOMId" />
>     >     These seem to work well, but the second one is not nice in my
> IDE,
>     > compared
>     >     to support for the first one.
>     >
>     >     <instance id="regularId" localId="localId" />
>     >     This probably should be an error for the current implementation,
> as
>     > Harbs
>     >     has pointed out. But at the moment it is possible and 'id' wins.
>     >
>     >     What I am suggesting is that we reconsider to have only one 'id'
> and a
>     >     second boolean flag to 'switch' it to localOnly or not. This flag
>     > could be
>     >     'localId' or 'localIdOnly', whatever seems best - I will use
>     >     'localIdOnly'  below to differentiate from the above.
>     >
>     >     <Instance id="myLocalOnlyId" localIdOnly="true" />
>     >     <Instance id="myLegacyId" localIdOnly ="false" />
>     >     <Instance id="myId"  />
>     >
>     >     By default 'localIdOnly' would be false when it is not
> specified, so
>     > the
>     >     same behaviour as it is now - the 3rd case above.
>     >     But I think it might be helpful to have the option to have a
> global
>     > config
>     >     for this so you could do a global default as a compiler setting
> to
>     > set it
>     >     to true by default - e.g. like 'ignore coercion' is set up iirc.
> This
>     > might
>     >     suit some people who prefer to 'start with things off and switch
> them
>     > on
>     >     only if needed'.
>     >     localIdOnly in the examples above is a compile time mxml-only tag
>     > setting
>     >     and is not propagated to the instantiated components, so it is
> not a
>     > real
>     >     value assignment to the instance and does not exist as a
> property on
>     > the
>     >     instances.
>     >
>     >     What this could mean: All IDEs still see the id as 'normal'
> legacy use
>     > -
>     >     for code completion, bindings, script block references etc. The
> new
>     >     'unknown'  localIdOnly boolean attribute is the only thing that
> IDEs
>     > would
>     >     need to special-case, which I think should be easier than the
> localId
>     >     string variation (I assume).
>     >
>     >
>     >
>     >     On Fri, Nov 2, 2018 at 10:01 PM Piotr Zarzycki <
>     > piotrzarzycki21@gmail.com>
>     >     wrote:
>     >
>     >     > Hi Greg,
>     >     >
>     >     > I'm really happy that you are helping Carlos with that! He may
> move
>     > forward
>     >     > much faster. I just have question to following:
>     >     >
>     >     > "-My understanding is that best practice is to avoid multiple
>     > identical ids
>     >     > in the browser, irrespective of whether the browser is
> forgiving of
>     > that or
>     >     > not. If so, it might be good to have at least an option to set
> the
>     > default
>     >     > implementation to support 'best practice' (DOM ids 'off' by
> default,
>     > 'on'
>     >     > explicitly, to avoid 'duplicate ids by accident'). Maybe some
> sort of
>     >     > import wizard for a legacy flex project could do something
> like this
>     > in an
>     >     > IDE by default though. But it could be a compiler config thing
> too
>     >     > perhaps."
>     >     >
>     >     > Does your idea is saying that if I have some Flex app or even
> write
>     > some on
>     >     > my own setting that option to ON - change the  way how things
> are
>     >     > outputting after compilation ? Do you mean that:
>     >     >
>     >     > <Button id="myid" /> - Option is ON
>     >     >
>     >     > output will be:
>     >     >
>     >     > <Button localId="myid" />
>     >     >
>     >     > I'm sorry if I misunderstand you completely :)
>     >     >
>     >     > Thanks,
>     >     > Piotr
>     >     >
>     >     > pt., 2 lis 2018 o 08:31 Greg Dove <greg.dove@gmail.com>
> napisał(a):
>     >     >
>     >     > > In collaboration with Carlos, I worked on a compiler fix for
> some
>     > issues
>     >     > > identified with localId in the javascript output. I pushed
> that a
>     > short
>     >     > > while ago. This fixes
>     >     > > -binding into the localId (in my local test cases) and
>     >     > > -some occasional issues with referencing the instance from
> within
>     > script
>     >     > > blocks in release (minified) code.
>     >     > > Or at least, it does so for the cases I have been testing. If
>     > anyone else
>     >     > > sees remaining issues with this feature that need more
> attention,
>     > please
>     >     > > let me know.
>     >     > >
>     >     > > Now on to the 'subject' :
>     >     > > As part of 'getting familiar' with this I went back to read
> old
>     > threads
>     >     > > about 'id v.s localId'.
>     >     > > I *think* these [1] [2] were the main ones, but maybe I
> missed
>     > some other
>     >     > > discussions.
>     >     > >
>     >     > > After reading these, I wondered if anyone had changed their
> views
>     > about
>     >     > the
>     >     > > implementation as it is, after having used it for a while.
>     >     > >
>     >     > > It may be too late to change things, but here are my quick
>     > thoughts about
>     >     > > this:
>     >     > > -My understanding is that best practice is to avoid multiple
>     > identical
>     >     > ids
>     >     > > in the browser, irrespective of whether the browser is
> forgiving
>     > of that
>     >     > or
>     >     > > not. If so, it might be good to have at least an option to
> set the
>     >     > default
>     >     > > implementation to support 'best practice' (DOM ids 'off' by
>     > default, 'on'
>     >     > > explicitly, to avoid 'duplicate ids by accident'). Maybe
> some sort
>     > of
>     >     > > import wizard for a legacy flex project could do something
> like
>     > this in
>     >     > an
>     >     > > IDE by default though. But it could be a compiler config
> thing too
>     >     > perhaps.
>     >     > >
>     >     > > -I can't think of a scenario where I would want to set both
> id and
>     >     > localId
>     >     > > at the same time or what doing so would mean. Either you
> want to
>     > set the
>     >     > > DOM id or you don't, in which case missing id and defined
> localId
>     > is more
>     >     > > like a boolean for not setting DOM id (the implementation is
> not,
>     > but to
>     >     > me
>     >     > > it seems that it could -maybe should- be). Maybe I am missing
>     > something
>     >     > > here.
>     >     > >
>     >     > > -'id' is the basis for code completion/intelligence in legacy
>     > IDEs. Using
>     >     > > 'localId' means this does not work in the legacy IDEs and
> newer
>     > IDEs need
>     >     > > to add custom support for it. Anything that keeps 'id' as the
>     > primary
>     >     > local
>     >     > > identifier seems like the best way to get more life out of
> legacy
>     > IDEs.
>     >     > >
>     >     > > So to me, the simplest option seems to be more along the
> lines of
>     >     > >
>     >     > > <Instance id="myLocalOnlyId" localId="true" />
>     >     > > <Instance id="myLegacyId" localId="false" />
>     >     > >
>     >     > > Semantically it is probably better as 'localIdOnly' for the
> boolean
>     >     > > setting, but 'localId' is shorter (which is perhaps better).
>     >     > >
>     >     > > In this case, you get more mileage from older IDEs, and a
> simpler
>     >     > > implementation for updating IDEs to handle the extra
> mxml-only
>     > boolean
>     >     > > setting. In simple terms everything else works the same so
> the
>     > IDEs still
>     >     > > work for code intelligence.
>     >     > >
>     >     > > An unspecified 'localId' boolean in mxml would currently be
> the
>     > same as
>     >     > > false, but could possibly have a global configuration
> default -
>     > not sure
>     >     > > about that, but maybe it could be useful.
>     >     > >
>     >     > > If there is an issue with styling on the swf side with valid
>     > multiple ids
>     >     > > vs. html, then I think the swf side could perhaps be
> outlawed in
>     > favour
>     >     > of
>     >     > > best practice for html. Too much? :)
>     >     > >
>     >     > > Anyhow, I am just raising this now in case anyone else has
> changed
>     > their
>     >     > > thinking after using it as-is for a while, and before it
> gets too
>     > late to
>     >     > > consider changing it (if it is not already too late).
>     >     > > If there is some consensus to change this, I am happy to
> work on
>     > it.
>     >     > >
>     >     > >
>     >     > >
>     >     > > 1.
>     >     > >
>     >     > >
>     >     >
>     >
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fapache-flex-development.2333347.n4.nabble.com%2FFlexJS-MXML-ids-and-classNames-td54361i40.html%23a63276&amp;data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&amp;sdata=IFNLh1z1tuEan3aBPwoY0pDoK%2F61fZiaIlruWymJBpg%3D&amp;reserved=0
>     >     > > 2.
>     >     > >
>     >     > >
>     >     >
>     >
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fapache-flex-development.2333347.n4.nabble.com%2FFlexJS-MXML-ids-and-classNames-td54361i60.html%23a63919&amp;data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&amp;sdata=A4TndNuY1ybB3mKp%2BRQAIRuHZNvf9FLzU0bBI0nlnR4%3D&amp;reserved=0
>     >     > >
>     >     >
>     >     >
>     >     > --
>     >     >
>     >     > Piotr Zarzycki
>     >     >
>     >     > Patreon: *
>     >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.patreon.com%2Fpiotrzarzycki&amp;data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&amp;sdata=6OMfqKGfLYK12c8aac%2FY1XmDdbHff7lyZLoPu0emmwM%3D&amp;reserved=0
>     >     > <
>     >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.patreon.com%2Fpiotrzarzycki&amp;data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&amp;sdata=6OMfqKGfLYK12c8aac%2FY1XmDdbHff7lyZLoPu0emmwM%3D&amp;reserved=0
>     > >*
>     >     >
>     >
>     >
>     >
>
>
>

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