Extract file system based functions into template loaders #377
Conversation
|
In the test you use
What would this look like on the client (browser)? Does this also address "include" in the browser or just "extends"? |
|
MemoryLoader must work with both extends and imports in browser. It just emulates file system via predefined dictionary: templates = {
'page.html': '<html>{% include "content.html" %}</html>',
'content.html': 'Hello {{ name }}!'
};But It still needs to be improved for enabling relative paths (like "../base.html"). |
|
I was about to implement something similar yesterday right before i saw your patch. One minor problem is that the loaders are not exported and not accessible when used with requirejs. The bigger problem is, that when you try to use requirejs to load templates that extend other templates, swig calls getParents() which in turn calls parseFile(). I am not sure at this point if its feasible to clobber swig with callback-stuff to make it work with asynchrously loaded templates, or if it makes more sense to just set up a grunt task to collect and maybe even precompile all templates and use something like the MemoryLoader. Maybe @paularmstrong has some feedback if he would accept a patch that makes swig work with templates loaded with requirejs, even if it would mean passing on a callback to getParents/parseFile. P.S.: Of course we could make every template, including its complete dependency-tree, a module dependency. But i'd rather not trace all dependencies by hand every time i want to extend a template. |
done
Good idea I thought to add it too.
I see, it is a real problem, because requires to make most of the methods async. I can try to do that in separate branch. |
|
i am really curious of what @paularmstrong has to say about this, as this (as far as i understand) is a fairly big change to swig. |
|
Really nice work, @MaratFM. I need some time to dig into this and potentially make some changes, unless you would prefer to make changes based on my requests.
We should add in the I'm sure we can pick at this forever, but those are a couple things that we should definitely take care of before merging. Sidenote: I'm not sure why tests are suddenly failing. As long as we make them pass when working locally, we should be fine. |
|
a couple of hours ago i did not see any failing tests. altough there were some conflicts i resolved in https://github.com/AffiliPRINT/swig/commit/5e8e745ab50334e05d1766ffa6f944f35a9aec46 |
|
I've already merged this locally and am taking it over with some changes and adding documentation... more to come soon... |
I hit the same issue as #370 when I tried to use swig in browser. Precompiling templates is not useful in some cases and does not work with latest code. That is why I tryed to fix it and and add some flexibility.
Loaders provides interface for extending swig for using different template sources. By default FileSystemLoader will be used and it acts as current swig implementation. As an example I added MemoryLoader which might be used in browser. It loads templates from predefined hash object.
I know that my pull request needs to be updated with more documentation and tests, but I want to know you opinion about it before.
Thank you