[Proposal] old() helper to return empty string if present but null

[imliam]

If you submit a form with an empty input, its name will be present in the session data session('_old_input')[$fieldName] with the value of null instead of (what I expected to be) an empty string ''.

Unfortunately, this means that when displaying old values for a form using the old() helper function, validation fails and you direct the user back to the form filling out old values, the form will not have the same values as the user just submitted because the default value will be used instead.

We can get around this manually by checking if the value is present in the session but has an explicit value of null and returning an empty string.

if (array_key_exists($fieldName, session('_old_input', [])) && session('_old_input')[$fieldName] === null) {
    return '';
}

// Existing old() behaviour...

If this were introduced, the default parameter would only be used as a fallback if the field name is not present in the request and empty fields would be respected.

I think the current behaviour is especially bad on long forms where it may not be immediately obvious that a field was changed between page loads, or even which one.

I figure this change would break existing apps so if it were to be introduced should be in 7.0 at the earliest.

[drbyte]

That conversion to null is done by the default https://github.com/laravel/laravel/blob/master/app/Http/Middleware/TrimStrings.php middleware. You can customize that file to add additional fields which you wish it to not "trim and treat empty as null".

[imliam]

Configuring the middleware to handle every single input field that could be in a form in your app seems a little excessive of a solution, even if it would work.

My concern here is more about the default experience (for both the developers and users of an app) just doesn't line up with how you might expect a form with validation issues to display the old fields.

[imliam]

My description didn't make sense to someone in the Discord, so figured I'd post an explanation of what I mean here:

---

Let's say you have an account settings page where your user can change their name and set bio text for themselves.

<input type="text" name="username" value="{{ old('username', $user->username) }}">
<input type="text" name="bio" value="{{ old('bio', $user->bio) }}">

The user fills this out, enters their bio details and saves it. All fine and dandy.

Now the user wants to delete their bio and change their name - which should be allowed - in this application, an empty bio is perfectly fine.

However, they submit their form and get a validation error on the username - so they get directed back to the form with the updated username they put in, but the old bio information - not the empty string they made it on the previous page.

[drbyte]

I see your point.

I wasn't suggesting that you whitelist every field, but rather just the occasional one where you know you don't want that middleware updating.

It's also important to note that if someone disables the Trim middleware, then it's also not going to convert that input value to null, so there'd be a need to accommodate all cases.

Here's the code that I think you're asking to change:

    /**
     * Retrieve an old input item.
     *
     * @param  string|null  $key
     * @param  string|array|null  $default
     * @return string|array
     */
    public function old($key = null, $default = null)
    {
        return $this->hasSession() ? $this->session()->getOldInput($key, $default) : $default;
    }

What are you wanting to change it to?

[imliam]

I guess I would like to change that getOldInput method from what it is now:

    public function getOldInput($key = null, $default = null)
    {
        return Arr::get($this->get('_old_input', []), $key, $default);
    }

To something that handles this scenario:

    public function getOldInput($key = null, $default = null)
    {
        $old = $this->get('_old_input', []);

        if (array_key_exists($key, $old) && is_null($old[$key])) {
            return '';
        }

        return Arr::get($old, $key, $default);
    }

[functioneelwit]

I had a similar issue. Isn't removing the \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull Middleware from the kernel just solving this problem or am i missing something here?

Maybe it helps someone else.

/**
     * The application's global HTTP middleware stack.
     *
     * These middleware are run during every request to your application.
     *
     * @var array
     */
    protected $middleware = [
        \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
        \Illuminate\Foundation\Http\Middleware\ValidatePostSize::class,
        \App\Http\Middleware\TrimStrings::class,
        // \App\Http\Middleware\ConvertEmptyStringsToNull::class,
    ]

In my case overwrote the default middleware to specify allowed fields.

namespace App\Http\Middleware;

use \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull as BaseConverter;

class ConvertEmptyStringsToNull extends BaseConverter
{
    /**
     * The names of the attributes that should not be converted to null.
     *
     * @var array
     */
    protected $only = [
        'phone',
        'mobile',
    ];

    /**
     * Transform the given value.
     *
     * @param  string  $key
     * @param  mixed  $value
     * @return mixed
     */
    protected function transform($key, $value)
    {
        if (in_array($key, $this->only)) {
            return $value;
        }

        return is_string($value) && $value === '' ? null : $value;
    }
}

[Loots-it]

I guess I would like to change that getOldInput method from what it is now:

    public function getOldInput($key = null, $default = null)
    {
        return Arr::get($this->get('_old_input', []), $key, $default);
    }

To something that handles this scenario:

    public function getOldInput($key = null, $default = null)
    {
        $old = $this->get('_old_input', []);

        if (array_key_exists($key, $old) && is_null($old[$key])) {
            return '';
        }

        return Arr::get($old, $key, $default);
    }

I think this would be a great idea. Although it is a breaking change. Removing the middleware from the kernel is only a solution if you want to control the conversions yourself. Maybe adding an extra boolean argument $transformNullToEmptyString is a possible solution that wouldn't break existing code?

[phantomaxe]

https://github.com/phantomaxe/laravel-project.git