Skip to content
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

Fix strict_variables issues #674

Merged
merged 2 commits into from
Nov 1, 2019
Merged

Fix strict_variables issues #674

merged 2 commits into from
Nov 1, 2019

Conversation

toptalo
Copy link
Contributor

@toptalo toptalo commented Oct 29, 2019

Makes it to match twigfeddle errors output

Example of new errors:

  • Variable "foo" does not exist.
  • Key "key" for object with keys "a, b, c" does not exist.
  • Key "key" does not exist as the object is empty.
  • Key "10" for array with keys "0, 1, 2" does not exist.
  • Key "10" does not exist as the array is empty.

Live example: https://twigfiddle.com/yfnyxt
Tip: you need to switch main template to see different output

Closes #629

Example of new errors:
- 'Variable "foo" does not exist.'
- 'Key "key" for object with keys "a, b, c" does not exist.'
- 'Key "key" does not exist as the object is empty.'
- 'Key "10" for array with keys "0, 1, 2" does not exist.'
- 'Key "10" does not exist as the array is empty.'
@RobLoach
Copy link
Collaborator

Looking at the Twig PHP options, it uses strict_variables...

https://twig.symfony.com/doc/2.x/api.html

strict_variables boolean

If set to false, Twig will silently ignore invalid variables (variables and or attributes/methods that do not exist) and replace them with a null value. When set to true, Twig throws an exception instead (default to false).

Unsure if it's best to go with the JavaScript option convention, or rolling with the one PHP uses.

@toptalo
Copy link
Contributor Author

toptalo commented Oct 29, 2019

I can revert this rename

@toptalo
Copy link
Contributor Author

toptalo commented Oct 29, 2019

I have an idea: what if we save strict_variables options param and rename all inner variable to camel case variant.

So here https://github.com/twigjs/twig.js/blob/master/src/twig.exports.js#L22
will be strictVariables: params.strict_variables || false,

@RobLoach
Copy link
Collaborator

That's a great idea... Would keep backward compatibility too 👍

…zation parameters to maintain backward compatibility
@RobLoach RobLoach changed the title Fixes strictVariables errors Fix strict_variables issues Nov 1, 2019
@RobLoach RobLoach merged commit 0badc3a into twigjs:master Nov 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strict_variables: true not working correctly
2 participants