-
Notifications
You must be signed in to change notification settings - Fork 274
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
Regression: Enabled strict_variables throw unnecessary errors #679
Comments
Sorry, I don't know how to fix it |
I tried to fiddle with the code a bit but didn't come far.
This means the exemption of the strict_variable check has to be stored in an isolated scope. As I'm very unfamiliar with the code my attempts to access the scope failed. {
type: Twig.expression.type.variable,
// Match any letter or _, then any number of letters, numbers, _ or -
regex: /^[a-zA-Z_]\w*/,
next: Twig.expression.set.operationsExtended.concat([
Twig.expression.type.parameter.start
]),
compile: Twig.expression.fn.compile.push,
validate(match) {
return (Twig.expression.reservedWords.indexOf(match[0]) < 0);
},
parse(token, stack, context, nextToken) {
const state = this;
var preprocess = function(token, state, nextToken) {
// If this is a check to see if the variable exists disable
// the strictVariable behavior.
if (nextToken.type === 'Twig.expression.type.test' && nextToken.filter === 'defined') {
state.template.options.ignoreStrictCheck = true;
}
return context[token.value];
}
// Get the variable from the context
return Twig.expression.resolveAsync.call(state, preprocess, context, [token, state, nextToken])
.then(value => {
if (state.template.options.strictVariables && !state.template.options.ignoreStrictCheck && value === undefined) {
throw new Twig.Error('Variable "' + token.value + '" does not exist.');
}
// If this was an overruled handling register the
// variable in the context.
if (state.template.options.ignoreStrictCheck && value === undefined) {
// Create dummy variable and
context[token.value] = '';
state.template.options.ignoreStrictCheck = false;
}
stack.push(value);
});
;
}
}, I've added a preprocess function to I leave it at that for the moment but we'll see if I can continue at some point where I've more free time to pursue this. |
Ran into the same problem. I use |
Is there any news on this?
Could the commit that introduced the regression be reverted? EDIT: I looked into it a bit and simply reverting the PR wouldn't help (the strict mode is not very useful without it). It doesn't look like there is an easy fix. I'm going to disable |
same problem here |
I found a solution (crutch) how to not throw an error if next token after variable is test to defined. In case when after undefined variable goes filter // Get the variable from the context
return Twig.expression.resolveAsync.call(state, context[token.value], context)
.then(value => {
const nextFilterDefined = token.next && token.next.filter === 'defined';
if (state.template.options.strictVariables && value === undefined) {
if(!nextFilterDefined) {
throw new Twig.Error('Variable "' + token.value + '" does not exist.');
} else {
context[token.value] = false;
}
}
stack.push(value);
}); Here I add token = tokens.shift();
token.next = tokens[0];
tokenTemplate = Twig.expression.handler[token.type]; |
Any idea when this can be merged and released? This bug makes strict_variables kind of useless :( |
This is just a bug report, not an actual fix for it. |
The changes from #629 seem to throw unnecessary errors.
Following template should always run even if
lie
isn't defined andstrict_variables
is enabled - just the output should change:The {{ baked_good }} is a{% if lie is defined and lie %} lie {% else %} reality {% endif %}!
However, with
strict_variables
enabled it fails withTwigException: Variable "lie" does not exist.
Running Example: https://jsfiddle.net/jsz9uy3h/6/
If you use https://cdn.jsdelivr.net/npm/[email protected]/twig.min.js in above fiddle it works.
For the moment I can only think of introducing context awareness into the
parse
function ofTwig.expression.type.variable
. However this seems like a good way to clutter things :|The text was updated successfully, but these errors were encountered: