django / django/new-features

Add more expressive callable to replace `string_if_invalid`

Open
#154 1 comment 9 reactions 0 assignees View on GitHub
Templates
Dominant language
No language data
Stars
188
Forks
7
PR merge metrics
No merged PRs in 30d

Description

### Code of Conduct

- [x] I agree to follow Django's Code of Conduct

### Feature Description

Provide an option to the DTL called `handle_invalid_variable` (or equivalent) that points to a callable which is called when the DTL encounters an “invalid variable” (primarily, variables that are missing from the context). This callable would act as a more expressive/flexible replacement for the existing `string_if_invalid` (replaced via a suitably non-disruptive deprecation path).

(I’m aware of [this existing feature-request,](https://github.com/django/new-features/issues/5) but think this proposal is sufficiently different as to require a separate thread / discussion).

### Problem

The primary problem this solves is of missing variables failing silently:
- [For a long time](https://groups.google.com/g/django-developers/c/8yMzgEWsLl4), there has been a consensus that missing variables failing silently cause more harm than good. However, many templates and third-party solutions have come to rely on this behaviour, making it difficult to change without breaking a large part of the eco-system.
- See also for example [this ticket](https://code.djangoproject.com/ticket/28618), [this forum thread](https://forum.djangoproject.com/t/raise-error-for-missing-variable-used-in-template/39776), [this mailing-list thread](https://groups.google.com/g/django-developers/c/zdULZcmAWNw)

There are a few other problems that the proposed feature solves as well though:
- Existing logging behaviour on missing variables is sometimes considered too loud (especially when missing variables are planned, and are therefore expected behaviour): https://code.djangoproject.com/ticket/28526
- For other people, such logging is an important feature. Regardless of your preference, there is currently no way to customise such logging.
- Logging behaviour is also inconsistent across invalid variables: https://code.djangoproject.com/ticket/24199.
- `string_if_invalid` can cause problems with many built-in templates, and despite warnings in the docs, `string_if_invalid` is often mis-used, as highlighted in [Adam Johnson’s blog-post](https://adamj.eu/tech/2023/08/09/django-perils-string-if-invalid/ ).
- The behaviour associated with missing variables is tightly coupled to the behaviour of other invalid variables (callables that change data, or require arguments etc.). It’s not obvious that these things should be treated the same way. For example, one might reasonably want missing variables failing loudly, but `DoesNotExist` errors to be silenced.

### Request or proposal

proposal

### Additional Details

When I started writing this feature-request I had in mind that the callable (from the feature description) would be a function that would take care of raising any errors, logging, other side-effects and typically return an empty string. In order to achieve maximum backwards-compatibility I **still** believe the default handler should indeed be a function. However thinking about [this suggestion](https://forum.djangoproject.com/t/raise-error-for-missing-variable-used-in-template/39776/11) further, I think an `InvalidVariableType` is the best solution when over-riding the default. Both callables can achieve similar things in terms of functionality, but I believe a `InvalidVariableType` delivers a much better developer experience via sub-classing, and aids in avoiding a few foot-guns. (I also think we should have a few built-in subclasses to act as “batteries” for common use-cases).

### Backwards compatibility
The proposed solution has a nice deprecation path, so backwards incompatibility shouldn’t be an issue:

During the initial release, we provide a default handler that mimics existing behaviour, so the change is non-breaking. At the point at which `string_if_invalid` is eventually removed, the original behaviour can still be maintained via a few lines of code if users wish (see [example 5](https://gist.github.com/tim-mccurrach/8ac32b41a856ced0ed5f94b80f508497#5-example-5-maintaining-current-behaviour-after-string_if_invalid-is-removed)).

Furthermore, for users wishing to adopt new behaviour on existing projects, there are several ways to roll-out the feature in a gradual fashion.

### Alternative approaches
This [forum thread](https://forum.djangoproject.com/t/raise-error-for-missing-variable-used-in-template/39776) (and other older threads referenced above) have a few alternative suggestions. I believe this proposed solution is flexible enough that all of the various desired behaviours could be achieved by providing a suitable handler.

I have written a [short "recipe-book"](https://gist.github.com/tim-mccurrach/8ac32b41a856ced0ed5f94b80f508497) that demonstrates how the proposed solution could be used. (Although I hope with suitably well chosen built-in subclasses, the need to write your own custom handlers would be rare).


### Is this a potential foot-gun?
Whilst very simple, the proposed solution isn’t particularly opinionated, and provides enough freedom for users to get themselves into trouble if they don’t appreciate some of the nuances of the DTL. I believe a good solution to this would be to document that `handle_invalid_variable` just needs to be a callable as described below, but to strongly encourage the use of `InvalidVariableType` in a how-to, which explains some of the pit-falls. Reading through most of the threads, I don’t find many of the proposals un-reasonable, and I think giving users freedom in this situation is a good thing, and with suitable documentation we can get the best of both worlds.

### Implementation Suggestions

[I have added a draft PR](https://github.com/tim-mccurrach/django/pull/1/changes), that shows how this can be achieved. The PR needs tests, docs, deprecation warnings, and probably a little bit of other love - but I’ve kept it intentionally small to make the change itself as clear as possible. (The change is actually quite simple).

The basic idea is that the callable should receive the following arguments:
- variable
- context
- reason
- kwargs

And the return value should be what is returned to the DTL.

I’ve also included a `BaseInvalidVariableType` that has a few properties and methods that can be used to make writing a handler easier.

I have click tested it with a few projects (trying it with all of the handlers in the recipe book) and everything seems to work nicely

Any thoughts and feedback are very much welcome :)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.