Skip to content

Conversation

@sileence
Copy link
Contributor

@sileence sileence commented Apr 5, 2020

The fix to escape merged attributes with e creates an unexpected behaviour when merging attributes with null or boolean values, for example:

        $bag = new ComponentAttributeBag([]);
        
        echo $bag->merge([
            'required' => false,
            'readonly' => null,
            'disabled' => true,
        ]));

Will print required="" readonly="" disabled="1" instead of disabled="disabled"

This is also inconsistent with the result that we'd get if we create a new instance of ComponentAttributeBag passing the same array as the first argument of the constructor:

new ComponentAttributeBag([
    'required' => false,
    'readonly' => null,
    'disabled' => true,
])

Will print disabled="disabled" and I think this is the right result.

Therefore I'm submitting this PR to avoid escaping boolean or null values so we get the same result in both cases.

@taylorotwell taylorotwell merged commit ff1a13e into laravel:7.x Apr 6, 2020
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.

2 participants