templates/modules.js.tmpl - Issue 29363565: Issue 4552 - Drop jshydra dependency (buildtools)

Delta Between Two Patch Sets: templates/modules.js.tmpl

Issue 29363565: Issue 4552 - Drop jshydra dependency (buildtools) (Closed)

Left Patch Set: Created Nov. 18, 2016, 5:20 p.m.

Right Patch Set: Check module exists before auto-loading it Created Nov. 30, 2016, 3:24 p.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

Left: Side by side diff | Download
Right: Side by side diff | Download

LEFT	RIGHT
1 {%- if args["module"] -%}	1 {%- if args["module"] -%}
kzar 2016/11/18 17:25:48 I went without indentation since it made the rende I went without indentation since it made the rendered output nicer. Otherwise I had to pick between no newline and newline with indentation spaces and that made things ugly fast.
2 {%- for module_name, script in modules -%}	2 {%- for module_name, script in modules -%}

3 require.modules[{{ module_name\|json\|safe }}] = function(module, exports, require )	3 require.modules[{{ module_name\|json }}] = function(module, exports)
Wladimir Palant 2016/11/30 11:39:32 No point passing in require as parameter, the glob No point passing in require as parameter, the globally defined require function will do for now. kzar 2016/11/30 14:29:45 Done. Show quoted text On 2016/11/30 11:39:32, Wladimir Palant wrote: > No point passing in require as parameter, the globally defined require > function will do for now. Done.
4 {	4 {

5 {{ script\|safe }}	5 {{ script }}
Wladimir Palant 2016/11/30 11:39:32 Enabling autoescaping and then marking each variab Enabling autoescaping and then marking each variable as safe sort of defeats the purpose of it ;) kzar 2016/11/30 14:29:44 Lol good point, done. Show quoted text On 2016/11/30 11:39:32, Wladimir Palant wrote: > Enabling autoescaping and then marking each variable as safe sort > of defeats the purpose of it ;) Lol good point, done.
6 return exports;	6 return module.exports;
Wladimir Palant 2016/11/30 11:42:14 This should return module.exports, in case the mod This should return module.exports, in case the module decides to replace it by its own object (which is valid with CommonJS). kzar 2016/11/30 14:29:44 Done. Show quoted text On 2016/11/30 11:42:14, Wladimir Palant wrote: > This should return module.exports, in case the module decides to replace it by > its own object (which is valid with CommonJS). Done.
7 };	7 };

8	8

9 {% endfor -%}	9 {% endfor -%}

10 {% for module_name, script in modules -%}	10 {%- set modules = dict(modules) -%}
Wladimir Palant 2016/11/30 11:39:32 This should be {%- for ... -%}, no point adding an This should be {%- for ... -%}, no point adding an extra newline here. kzar 2016/11/30 14:29:45 Done. Show quoted text On 2016/11/30 11:39:32, Wladimir Palant wrote: > This should be {%- for ... -%}, no point adding an extra newline here. Done.
11 {%- if module_name in args["autoload"] -%}	11 {%- for module_name in args["autoload"] -%}
Wladimir Palant 2016/11/30 11:39:32 This should not iterate over modules again, rather This should not iterate over modules again, rather over args["autoload"]. The autoload order should logically be defined by the autoload argument, not by the module order. kzar 2016/11/30 14:29:44 Done. Show quoted text On 2016/11/30 11:39:32, Wladimir Palant wrote: > This should not iterate over modules again, rather over args["autoload"]. The > autoload order should logically be defined by the autoload argument, > not by the module order. Done.
12 require({{ module_name\|json\|safe }});	12 {%- if module_name in modules -%}

13 {% endif -%}	13 require({{ module_name\|json }});

14 {%- endfor %}	14 {% endif %}

	15 {%- endfor -%}

15 {%- else -%}	16 {%- else -%}

16 {%- for module_name, script in modules -%}	17 {%- for module_name, script in modules -%}

17 {{ script\|safe }}	18 {{ script }}

18 {%- endfor -%}	19 {% endfor -%}

19 {% endif %}	20 {%- endif -%}
Wladimir Palant 2016/11/30 11:39:32 This should be: {% endfor -%} {%- endif -%} In o This should be: {% endfor -%} {%- endif -%} In other words, there should be a newline after each script, not only newlines (currently two) at the end of file. kzar 2016/11/30 14:29:44 Done. Show quoted text On 2016/11/30 11:39:32, Wladimir Palant wrote: > This should be: > > {% endfor -%} > {%- endif -%} > > In other words, there should be a newline after each script, not only newlines > (currently two) at the end of file. Done.
LEFT	RIGHT