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%7Ccaccb21e810744f661c108d640e68903%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767756277520274&sdata=aRsxjfUPFPtNUE%2Fgp8n0EN7Y9JTilQZkvfGSt2v6iHc%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%7Ccaccb21e810744f661c108d640e68903%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767756277520274&sdata=sYFsiUSsyIPcaEbgNpQWXjUAyXfPFZovgWzTBCZ7a14%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%7Ccaccb21e810744f661c108d640e68903%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767756277520274&sdata=Ta4WKVB40dz%2B088PuNHPXE2J%2Bq1kytH%2FpU%2BoxgYgZlQ%3D&reserved=0 > > < > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.patreon.com%2Fpiotrzarzycki&data=02%7C01%7Caharui%40adobe.com%7Ccaccb21e810744f661c108d640e68903%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C0%7C636767756277520274&sdata=Ta4WKVB40dz%2B088PuNHPXE2J%2Bq1kytH%2FpU%2BoxgYgZlQ%3D&reserved=0 > >* > > > > >
