search

trueadm / t7

Lightweight virtual DOM templating library
900 stars 31 forks source link

Adds support for splats #24

Closed a-s-o closed 8 years ago

a-s-o commented 8 years ago

Hello,

Here is my PR for the splat feature as discussed in issue #23. I have given it a shot. Wow, that stringed function approach in this project. I assume it is for that sweet performance, but it is a brain scrambler.

To summarize, this modification not tracks all the attributes in an array called assignments on the root, instead of root.attrs as insertion order becomes important. The array contains a tuple [name, val] in case of normal attributes and just the val to be assigned in case of a splat.

Later on it is up to a new function called joinAttrs to put this all together and it does that by sequencing the assignments into a single Object.assign call.

I also had to abstract the iteration over the attributes so the old buildAttrs and buildInfernoAttrs variants were inlined into the joinAttrs function. In case of inferno the second argument of joinAttrs receives a bound callback where all the template sideeffect can happen. I have called this the infernoTemplateHelper.

All tests pass, including a new one for the splats. I also added a description to the readme.

Hope that all makes sense. Please feel free to ask if something is unclear.

trueadm commented 8 years ago

@a-s-o This is great work! Thank you for taking the time to do this. In regards to the Inferno related parts of the codebase – I think it's safe to completely remove them as t7 hasn't supported Inferno for quite some time. I'm not sure if you would feel up to removing this deadcode from your PR in order to tidy up the t7 codebase or if it should be a separate PR. What are your thoughts on this? Thanks again :)

a-s-o commented 8 years ago

Oh, I see. That is why i was running into some bugs; I am actually using this with Inferno. Would you consider continuing and updating inferno support? I like the hooks approach and am starting to use it more than react/mithril.

If you are worried about filesize or coordinating updates, how about we switch to a plugin system for t7 outputs and maintain the inferno "plugin" in the main inferno repo as an alternative to inferno-create-element

trueadm commented 8 years ago

@a-s-o There was a branch in t7 where it was re-wrote to be plugin-able but development stopped due to lack of demand. This is definitely something I wanted to revisit for Inferno 0.8 and its new template approach which would tie nicely into how ES2015 tagged templates work. I'm not sure what the best plan of action is given you want Inferno support for t7 in 0.7 – can you wait or do you want to spike your own inferno-t7 like extension module like you mentioned?

a-s-o commented 8 years ago

I can totally wait. I wasnt aware of the inferno 0.8 branch. I will check that out. Thanks.

trueadm commented 8 years ago

I'll merge this PR anyway for people who might want to use your work with other vDom libraries. Thank you :)