Refactor and fix load()

[ghost]

• [x] This is to fix an issue (caching on, name given, but no file and no str)
• [x] I ran npm run build and npm run test

Summary

This fixes a minor issue (error is not returned from load when caching is enabled, name is given, but no template). It also refactors and simplifies the function a bit.

[nebrelbug]

Hi @joergleis, thanks for the pull request - this looks great! Just one issue: there should be an else if (name) block, so if the user passes in a name but not a string, the function will still work.

[nebrelbug]

I'm out of town until tomorrow, so I'll merge it (after a few more tests) then.

Thanks @joergleis!

[nebrelbug]

Thanks @joergleis for the pull request! Sorry it took so long to merge - I had an unexpected engagement come up. I'll publish a new version on NPM this afternoon.

[ghost]

Great! :)

[ghost]

What do you think about these (for the next major release, possibly):

1. Split up load() into two functions, one for strings, one for files -- i.e., use renderFile to compile, cache and render files, and use another for string templates. The code size would get slightly smaller and the renderFile function could be omitted for a browser dist.

2. Allow $name for file templates.

[nebrelbug]

@joergleis I just published version 7.4.0 with your changes 👍

[nebrelbug]

I like both of those ideas.

Right now, there's a separate runtime-utils.js file for the browser, but it doesn't have support for caching, so I think a better load function would be in order.

I like the idea of allowing $name for file templates too. My goal is to have a loadTemplates function that accepts an object containing:

{
"templates": {
  "mytemplate": function (str, options) {...},
  "myothertemplate": function ...
},
"partials": {
  "mypartial": function ...,
  "mysecondpartial": function ...
},
"layouts": {
  "mylayout": function...
}
}

The load function would register additional templates by merging them with the object containing the templates, partials, layouts, etc. This would make it easier for plugins to change templates.

If we did this, your suggestion about having $name for file templates could be very nice.

[ghost]

I tried out splitting load() into two functions (see cad3fb69ceb0a1c884aa9bc4a73a65ea9aec6376). It saves about 2% on squirrelly.min.js and the gzip.

1. How about dropping the option of caching with str as name? The behavior is not documented right now, and I am not sure about a good use-case.

2. If we target v8, I suggest also unifying capitalisation and possibly naming: Render vs renderFile

3. Can you describe loadTemplates more in detail? E.g., the functions taking str and options. I can implement it, or at least adjust load and the proposed loadFile accordingly.

[nebrelbug]

Hi @joergleis, thanks for all of the great contributions & questions! I'm going to be without internet until tomorrow ☹️, so I'll respond then.

[nebrelbug]

@joergleis So sorry for the late response! Another trip came up, and my life has been very busy lately.

Regarding your suggestions:

1. I want to keep caching with str and name, to support caching for those who don't write their templates as specific files. For example, if somebody writes a template as a string in a JS file
2. I completely agree 👍
3. I'll write this in a new comment 😄

[nebrelbug]

Hi @joergleis! I just added the All-Contributors bot to this repository. May I add you to the README as a contributor?