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 errors caused in jinja StrictUndefined mode #2571

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pamelafox
Copy link
Contributor

Fixes #2522

This PR addresses errors that arise when running jinja in StrictUndefined mode (like mypy for Jinja!). There were different ways that I addressed the issues depending on the attribute:

  • extra_css/extra_js: I added these as attributes to the BaseView, since their usage and documentation implies that they should always exist as empty lists.
  • form.csrf_token: This is only defined if the dev is using SecureForm as the base class, which isn't always the case, so I used the pattern of "form.csrf_token is defined and form.csrf_token" which seems to be the recommended way in Jinja to ensure a variable exists before using it.
  • return_url: From what I could tell, this is only passed in sometimes? So I made an if to only use it if it exists. But I might be wrong about that.
  • widget.input_type: Only the classes that output HTML input fields have this defined. I considered adding input_type: Optional[str] = None to all the other classes, but that would have been a larger change, so I added the "is defined" check to templates instead.
  • config.FLASK_ADMIN*: Since these will only be defined by devs who want to use that particular functionality, I used the "is defined" pattern for them.

Generally, for most cases, I ended up using the "is defined" pattern, but if you think there are more situations where .py changes would be warranted, let me know!


Fixes:

* Jinja templates can now be loaded in StrictUndefined mode.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if 1) this change warranted an entry,
2) this was the correct version header

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Multiple issues when using StrictUndefined in app.jinja_env.undefined
1 participant