New enyo.Control.applyStyle and enyo.Control.addStyles methods

I was wondering if someone from the enyo team could explain the idea behind the changed enyo.Control.applyStyle and enyo.Control.addStyles methods. In enyo 2.4, they were pretty similar. You pass one or more styles, the domStyles object got updated, and the new domStyles got converted back to cssText and applied to the node.

Now in enyo 2.5, applyStyle does a whole load of complicated things, including a scary looking regexp to update a style property which - i think - has replaced the old domStyles hash? In contrast, addStyles seems super simple again, doing none of the complicated Firefox-bug-evading regex-fu css rewriting ninja business seen in applyStyle.

So, what's up with that large difference? Are they now aimed at significantly different stages of a control's lifecycle? Or is one of them in need of an update?

Comments

  • Performance tests we executed internally proved that the string storage was significantly faster than hash-to-string conversion (and vice versa). The scary looking regular expression to maintain backward compatibility with the notion of adding styles to a control instance that was not backed by an actual DOM node. However, in nearly every case where this is happening it can probably be avoided and if at all possible - should be. The lifecycle of the control is most likely being used in an irregular fashion. I'm sure there are cases that break that mold, and that is why the regular expression exists. Just because it is big doesn't mean its scary ;) Read it! It actually makes perfect sense. If not for the need to deal with variable url expressions it would be so simple it hurts.
  • In that case, first of all, kudos for the speed improvement!

    The reason I call regexp scary is because almost every time I have ever tried to use regular expressions to do stuff that has to do with css or html there will be some odd edge-case that isn't covered and requires significant unexpected extra complexity to fix.

    Also, I might be wrong, but I suspect the current regex breaks if I put a url expression inside a url expression. An example of that particular case can be found in a bug-report I opened in jira a while ago (https://enyojs.atlassian.net/browse/ENYO-4129) where enyo's minifier breaks on exactly that one particular case.

    When I tried fixing that particular bug with regular expressions, complexity exploded and I quickly decided to ditch the regex approach entirely.

    Considering the whole applying css styles to a component that does not have a node yet is an edge-case and this might be something that can be overlooked, but it is something to consider.

    Either way, my main concern was the difference between the two methods. addStyles does not seem to care about the edge cases covered in applyStyle, even though they essentially do the same thing.
  • They are similar, but serve different purposes (its why they both exist). As for the regular expression, the case you're worried about won't exist in current code. You won't be passing less/css to a runtime function (or shouldn't be). And also note that the regular expression isn't doing anything fancy, as far as the URL portion is concerned, it merely matches the beginning and end of it and ignores the content. So, the only time it will "fall over" is if you have mal-formed or invalid syntax.
  • Allright, makes sense. I'm pretty sure there are valid ways to include a closing parenthesis and a semicolon inside a url expression, it would indeed likely only happen when people are trying to do things they should not be doing.

    I came across this change because I was wondering if I could easily refactor these two methods to allow updating the styles hash in enyo 2.4 without updating the dom styles and manually trigger the application of the changed the styles at a later point in time. I figured this could give more control over how many dom changes happen - and when they happen - as a possible way to enhance performance.

    When I compared it with 2.5.x to see how future-proof such changes would be I saw the new implementation. I suppose now that everything works with strings, the ability to control the application of new styles could be implemented even more easily.

    By the way, an idea that comes to mind as I was typing the above: perhaps it would be possible to 'lock' a component such that it prevent *any* changes to the dom, storing them in a buffer instead and explicitly allowing them to be applied at once at later point in time... hmm.. starts to sound a bit like react, doesnt it?
Sign In or Register to comment.