knockout-classBindingProvider icon indicating copy to clipboard operation
knockout-classBindingProvider copied to clipboard

Routing function with prefixing

Open mberkom opened this issue 12 years ago • 7 comments

I extended the getBindings method to call a getCleanPrefix function that recurses up the DOM tree looking for nodes with a data-class-prefix attribute and returning the complete prefix once it hits the <body> tag or an element with a data-class-prefix containing "$root".

The newly generated class prefix is then passed to the binding router and prepended to the class name before it attempts to look up the binding in the bindings object.

This allows you to write HTML like this:

<div data-class-prefix="$root.test">
    <div data-class-prefix="level1">
        <span data-class="test"></span>
        <span data-class="test2"></span>
    </div>
    <div data-class-prefix="level2">
        <span data-class="test"></span>
        <span data-class="test2"></span>
    </div>
</div>

instead of...

<div>
    <div>
        <span data-class="test.level1.test"></span>
        <span data-class="test.level1.test2"></span>
    </div>
    <div>
        <span data-class="test.level2.test"></span>
        <span data-class="test.level2.test2"></span>
    </div>
</div>

Technical Details

The getPrefix method decorates all child nodes of an element with a data-class-prefix attribute containing "$root". This is easiest to understand if you open up your inspector and look at the generated HTML. It adds the data-class-prefix attribute to all nodes, so that the second time a child element on the same branch goes to get its prefix, it doesn't have to recurse all the way up the tree again.

Knockout's ko.domData might be an effective way to clean things up, but I'm not sure it's the most straightforward thing to implement and might be a source of memory leaks if DOM elements are removed outside of Knockout.

Thoughts?

mberkom avatar Oct 12 '12 18:10 mberkom

@mberkom - still intend on looking through this soon. Thanks for submitting it.

rniemeyer avatar Oct 17 '12 02:10 rniemeyer

Sounds good. I just squashed all the commits together to make things easier to follow.

mberkom avatar Oct 17 '12 19:10 mberkom

@mberkom Hi Michael- I took a look at this a bit last weekend. Pretty interesting. I think that I would probably use the ko.utils.domData functions and assume that Knockout will be cleaning up the elements. It seems to work well.

I think though to keep this library a bit simpler for the time being, I would like to hold off on merging this in, although I certainly see the benefits of this approach.

rniemeyer avatar Oct 31 '12 03:10 rniemeyer

@rniemeyer Sounds good. I'm going to continue to use it in the project I'm working on. So far, the simple data attribute mechanism is working fine and doesn't annoy me much. It has the added benefit of being immediately obvious when you open up the inspector... However, I could see switching to ko.utils.domData in the future.

mberkom avatar Nov 01 '12 15:11 mberkom

@rniemeyer Would it be possible to reconsider what it'd take to get this feature included officially? I've found it to be incredibly handy and easy to use. However, there are a couple of areas that could certainly use more thought.

  1. How the prefixes are formatted - While you can replace the routing function with any implementation you want, you're currently restricted to formatting your class prefixes with the dot notation.
  2. How the prefixes are stored on nodes - Adding data-class-prefix attributes on every node it recurses is a little messy looking in the inspector. However, there are downsides to switching completely to ko.utils.domData. You lose the ability to see the complete binding path in the inspector and also increase the likelihood of memory leaks if someone doesn't clean things up properly.

For most Knockout developers doing simple projects, this library is probably overkill. On the other hand, those of us doing complex projects need every last bit of organization & re-usability that we can get.

mberkom avatar Nov 20 '12 18:11 mberkom

@mberkom - I am definitely willing to consider putting this type of functionality into the library. I do think that ko.utils.domData is a safe choice, given that KO will clean it up. However, since we only need to store a string, an attribute is also not a bad choice.

I will have to take another look through the code. It would be nice if we didn't need to have $root in the values.

rniemeyer avatar Nov 21 '12 02:11 rniemeyer

@rniemeyer Cool! Make sure to look at my latest commit description. It explains what I did.

Yeah, I'm not a fan of having $root in there, but it really speeds things up because once one branch of the tree has been traversed all the leafs and branches coming off that branch no longer have to traverse all the way up to the trunk but can stop the first time they encounter a class prefix containing $root.

mberkom avatar Nov 21 '12 03:11 mberkom