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 <[email protected]> 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" <[email protected]> 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 <[email protected]> > 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" <[email protected]> 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 < > > [email protected]> > > 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 <[email protected]> > 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&data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&sdata=IFNLh1z1tuEan3aBPwoY0pDoK%2F61fZiaIlruWymJBpg%3D&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&data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&sdata=A4TndNuY1ybB3mKp%2BRQAIRuHZNvf9FLzU0bBI0nlnR4%3D&reserved=0 > > > > > > > > > > > > > -- > > > > > > Piotr Zarzycki > > > > > > Patreon: * > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.patreon.com%2Fpiotrzarzycki&data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&sdata=6OMfqKGfLYK12c8aac%2FY1XmDdbHff7lyZLoPu0emmwM%3D&reserved=0 > > > < > > > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.patreon.com%2Fpiotrzarzycki&data=02%7C01%7Caharui%40adobe.com%7C1d7377f6700b4c018ec608d640ecc475%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767782987968136&sdata=6OMfqKGfLYK12c8aac%2FY1XmDdbHff7lyZLoPu0emmwM%3D&reserved=0 > > >* > > > > > > > > > > > >
