ckirkendall / kioo

Enlive/Enfocus style templating for Facebook's React and Om in ClojureScript.
Eclipse Public License 1.0
404 stars 39 forks source link

Compile time transforms #72

Open alesya-h opened 7 years ago

alesya-h commented 7 years ago

This allows having transforms that should be performed during compile-time. Main reason for this were that when you have components that are containers for other components you have great level of markup duplication when you'll never actually need it. Think of page with container with list of files. list container will contain all markup with sample files, it's container also will contain it, and page layout container will also contain it. With this changes you may have it like this:

(defsnippet file-component' "templates/profile.html" [:section.profile-main-section :#tab-files [:.col-xs-3.file-outer first-of-type]]
 [file local-state owner folder stats]
 {[:.file-content-type] {(kioo.common/content)
                         (kioo/substitute [category-label/category-label-component file])}

Above will apply (kioo.common/content) at compile-time, so resulting clojurescript for the component won't contain markup that would always be replaced by (kioo/substitute [category-label/category-label-component file]) at runtime.

(kioo.common/content) is the most immediatelly useful compile-time transform, so I've also added shorthand syntax for it (notice square brackets):

(defsnippet file-component' "templates/profile.html" [:section.profile-main-section :#tab-files [:.col-xs-3.file-outer first-of-type]]
[file local-state owner folder stats]
{[:.file-content-type] [kioo/substitute [category-label/category-label-component file]]

This does exactly the same thing as above, but arguably much cleaner.

Using above in our codebase have shrunk total compiled clojurescript from 14 MB to 7.3 MB.

ckirkendall commented 7 years ago

I like the idea here, need to spend a bit of time understanding it.

ckirkendall commented 7 years ago

There is currently the ability to do compile time transforms using the process-ast fn. You can see the pr here: https://github.com/ckirkendall/kioo/pull/65 and the original issue here: https://github.com/ckirkendall/kioo/issues/64

alesya-h commented 7 years ago

The thing you've referenced is a way to preprocess whole html snippet, not a way to apply kioo transformations to snippet's subtrees by selectors at compile time.

ckirkendall commented 7 years ago

You are correct but that is by design. To accomplish what you want the process-ast function could be an enlive transform that has selectors and handles that specific transform at that specific location in the tree. The idea here is that transforms at compile time are completely up to you and are not limited to kioo or any other type of transform.

alesya-h commented 7 years ago

With my approach one can use own function instead of kioo transform just as well — instead of (kioo.common/content) in the first example one may use arbitrary (foo.bar/baz). But with this pull-request the syntax is much cleaner, and you wouldn't need to duplicate selectors and use internal api to apply transformations to subtrees.

alesya-h commented 7 years ago

And behaviour from old pull-request may be duplicated as compile-time transform on [root] selector.

ckirkendall commented 7 years ago

I am not disagreeing with anything that you have said. There are limitations to what you propose because of how kioo works. In your setup you have to choose either a compile time transform or runtime transform at that spot or any children of that spot. That limitation worries me and I feel it is going to cause confusion that doesn't exist if we keep this completely separated. All that being said the syntax is nice and a simple macro to extract that out and create the process-ast function would probably be better than integrating this into the compiler directly. Not sure it would be part of Kioo main but a wrapper for those that prefer that syntax over process-ast directly.

alesya-h commented 7 years ago

have to choose either a compile time transform or runtime transform

No, the choice is between runtime and compile time plus runtime transforms. You always have runtime transform, but if you also need a compile-time transform, you wrap your transform with {compile-time-goes-here %}

ckirkendall commented 7 years ago

@alesguzik - I am referring to the fact that I can have both a runtime selector and compile time selector at the same point, unless I am misunderstanding your code. Also because you can't guarantee order of how this is applied you are also limited to not transforming anything matched inside the transformed ast.

alesya-h commented 7 years ago

I think most of the time selectors just don't overlap. When they do overlap you get exactly the same undefined behaviour, just in runtime. It's the issue with hash-map being used for the syntax of selector-transform pairs.

ckirkendall commented 7 years ago

That limitation is more than just because of the hash map. The hashmap was my way of conveying that order can not matter for kioo transforms. This is due to all the selectors being run at compile time. There is no way to run them efficiently at runtime to avoid this. I agree that most of the time runtime transforms don't overlap and that is why I thought it was a reasonable to build kioo. I don't think that compile time transforms and runtime transforms follow the same pattern. I think you could write a wrapper that provider this syntax around kioo but I don't think it is a good idea to add it to the base compiler. I could see a simple macro wrapper that pulled out the compiled time transforms and built a process-ast function using enlive.

alesya-h commented 7 years ago

I think ability to cut out inner pieces of subselectors at compile time is valuable for all kioo users, as it significantly reduces size of emited js. Ability to run any transform at compile-time was just the simplest way I found to implement this.

klimenko-serj commented 7 years ago

Hi guys! It is great ability to reduce traffic. I miss it in kioo. @ckirkendall please merge it!