Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile time transforms #72

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alesya-h
Copy link
Contributor

@alesya-h alesya-h commented Oct 25, 2016

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
Copy link
Owner

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

@ckirkendall
Copy link
Owner

There is currently the ability to do compile time transforms using the process-ast fn. You can see the pr here: #65 and the original issue here: #64

@alesya-h
Copy link
Contributor Author

alesya-h commented Nov 4, 2016

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
Copy link
Owner

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
Copy link
Contributor Author

alesya-h commented Nov 4, 2016

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
Copy link
Contributor Author

alesya-h commented Nov 4, 2016

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

@ckirkendall
Copy link
Owner

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
Copy link
Contributor Author

alesya-h commented Nov 4, 2016

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
Copy link
Owner

@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
Copy link
Contributor Author

alesya-h commented Nov 4, 2016

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
Copy link
Owner

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
Copy link
Contributor Author

alesya-h commented Nov 4, 2016

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants