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
Add option: Warning on unknown variables #599
Comments
Love this feature from other frameworks while developing locally, so +1 from me! I think it's important it has minimal to no impact on performance tho. Maybe even doable while keeping the core as is? E.g. by overriding some internal methods to enable this kind of behaviour while developing. |
Maybe a |
I'd prefer a hard error, so that, for instance, when using with Express it becomes a 500 response and not just a log somewhere while the end user is seeing an incorrectly rendered page (potentially very incorrectly rendered, depending on how the missing variable was supposed to be used); this could be even more useful in production than in local development, depending on how good your 500 page is vs. how bad the mis-rendered pages would be for your application's functionality. Use of sections could still allow templates to ignore missing variables even with hard errors for direct use. And any higher-level use that needs to log the issue would be in control of the logging system, so we wouldn't have to worry about, say, Mustache's internal warning mechanism clobbering output from test runners or things like that. I have a working prototype at https://github.com/ScottFreeCode/mustache.js, although I could use some help figuring out how to write a test for it. |
Hmm, so using the existence of an object as an Or only actual rendering of variables ( Edit: very cool feature that I would +1 to enable by default in a major in case it doesn't end up being a hassle. |
In my mind the first should be an error, the second a ‘warning’. Eventhough in the second case it might not technically break, it could have a huge impact on the page. Also, the other way around would be nice: unused variables… But that’s a lot more impact to make, I think! :D
|
@MatthijsZw I see. I just remembered you could store a Edit: My current position is that I'd prefer to Err on both. I prefer consistent behaviors, and you'd enforce the use of
I'm thinking that it would be cool to somehow get the Schema for the data a template is expecting, and use that to generate a GraphQL query... or something similar. |
On further reflection, I think part of the issue here is that missing data is invalid because -- and therefore only if -- the template is expecting that data. So, missing data that the template would just plain display is always invalid by that logic, but it's conceivable that a template might in one place expect a certain data to be available and branch on it as some kind of true/false flag (so it's invalid if it's not so much false as missing and therefore not meeting that expectation), but in another it might expect that data may or may not be available and branch on whether it is (in which case it's never invalid). In that perspective, using
What we need is two types of branches, for the different expectations on the part of the template. Which to my knowledge is unfortunate because the language-agnostic Mustache specification, while it allows for (but does not mandate) configuration of whether missing data is an error, as far as I know doesn't have anything for a different type of branch that would differ on this point. Hmm... On the flipside, I'm currently thinking that extraneous/surplus/unused data isn't really a matter of the data being invalid, it's a matter of the template being invalid if it doesn't use that data when the application/data/model expects it to be used. That is, if some stuff is potentially useable but the template can decide whether it's relevant, then it doesn't matter whether the template prints that stuff, but if some stuff truly needs to be displayed to the user, then if the template doesn't display it that's an error. As a sort of reversal, the expectation lies outside the template (in the model?) and the invalidity of failing to meet that expectation lies in the template. Probably best to tackle that separately, I'd figure. The above are, I suppose, strong opinions weakly held. |
IMO interpreting { name: null } that object has a A more appropriate check would be to check if a requested property has been defined like we do in mustache.hasProperty(). |
What I was trying to convey is that, if you branch depending on key
So, in this case, I would see benefits in throwing an Error. (trying to branch on a key that has a value of On the other case, trying to render either
Afaik, Mustache philosophy is not to use the models as-is, but to generate a 'view' from them. You could add
Hmm, I think it's pretty common not to use all the provided data. I threw the idea in mainly for tooling niceties, like the GraphQL query generation I suggested.
But who/what decides what "stuff truly needs to be displayed to the user"? The template writer I'm guessing? If you force people to use all data in the view, then you are forcing them to generate a tailored view for each template they intend to render. |
As a note: my use case was a manual setup without branching. I had ‘other people’ create the data (events) manually and in that case I had no way of verifying they filled in all the fields or made typos in the variable-names. My template would say “{{ event.name }} on {{ event.date }}”. In that case a missing value would create a horrible page. In this case ‘not used’ variables would be great to know as well, to double check for typos or for items they added thinking they would ‘magically’ appear on the page :) I’m sure this use case violates the ideology of Mustache somehow, but it’s a real scenario.
|
Both seem feasible. It could be useful for CI. |
I'd prefer seeing the parameter directly in the view for debugging. |
My use-case is a bit different: I use Mustache to transform Terraform templates using Gulp. A missing variable can cause instances to not boot correctly, especially when substituting in free-form strings. I came up with a quick monkey-patch to get this functionality but it's hardly ideal: var mustache = require("mustache");
var errors = [];
var lookup = mustache.Context.prototype.lookup;
mustache.Context.prototype.lookup = function(name) {
var value = lookup.bind(this)(name);
if (value === undefined) {
console.error("Unknown symbol", name);
errors.push(name);
}
return value;
}
var render = mustache.render;
mustache.render = function(template, view, partials) {
var result = render.bind(this)(template, view, partials);
if (errors.length > 0) {
throw {message: "Unknown symbols: " + errors.join(", ")};
}
return result;
} Notes:
However it worked well for my purposes and I'm sure someone can adapt it if required. |
Using mustache as a templating engine in any kind of configuration management environment requires hard errors on missing variables. I have a use case similar to @steverukuts, transforming kubernetes deployment documents. Missing variables are always errors in this use case. |
@stefaneg a couple of months after writing that in fact I discovered that Terraform supports configuration written in JSON format so now I use this instead of using Mustache. This is much better and more programmable. We've now deprecated the use of Mustache for this and it will be removed in the next revision of our deployment pipeline. Having looked at the Kubernetes deployment documentation I see that these are YAML files. Most programming languages have libraries that can read and write YAML so I suggest you do this instead. From my experiment, I learned that when you are trying to manipulate a machine readable format, you nearly always have a better option than Mustache. Please note: while I do not use Mustache for anything anymore, I still feel that this is a valid feature request. |
The solution is to use handlebars, which supports the same syntax, and also has a strict option which is exactly what is needed in this usecase. @steverukuts You are right about the manipulating machine readable format, if you have a predetermined manipulation that you need to support, and especially if you need to have some smarts built into it. For an open ended configuration tool, that becomes too inflexible very fast, hence the need for templating. Try writing a tool supporting inserting arbitrary values in arbitrary places and...you have a templating engine very soon. |
At work we've been using kontemplate for configuring Kubernetes resource the last few years. It's basically the go templating engine, with some handy functions from sprig and custom ones in that project. It has been made as a more lightweight approach than helm for example. In relation to the discussion above; it will also explode on unknown variables. |
This issue forced me into using handlebars as well. It's too bad this isn't supported in mustache. |
For anyone that doesnt want to/cant move to handlebars I've made a small package to add validation to mustache |
Just to note that I tried the above with a TypeScript project. It looks really promising, but my IDE couldn't find the types to enable me to use the package... I've logged it here: eliasm307/mustache-validator#1 |
This is now fixed, and mustache-validator is working very well for me. Thank you @eliasm307! |
Sometimes we make typos on variable names (even with autosuggest).
It would be great if there was a configuration so mustache-js would generate a warning on 'unknown' variables, instead of giving back an empty string (eventhough it is spec-compliant).
The Mustache man-page says:
By default a variable "miss" returns an empty string. This can usually be configured in your Mustache library. The Ruby version of Mustache supports raising an exception in this situation, for instance.
The text was updated successfully, but these errors were encountered: