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

Add rules while preserving integer rule keys inside the validator #39368

Closed
wants to merge 1 commit into from

Conversation

jeffreyzant
Copy link

This resolves #39362.

@taylorotwell
Copy link
Member

Could be a breaking change for some users.

@Tofandel
Copy link
Contributor

Tofandel commented May 18, 2024

I don't think array_replace_recursive is the right change instead we need a custom merge function, because array_merge_recursive appends ANY numeric key to the array, it doesn't check if the array is an assoc array first instead of preserving the keys, and in the case of a validator, it doesn't make sense to pass in an associative array

array_merge_recursive([], ['foo' => 'required', 4 => 'required', 8 => 'required']);
= [
    "foo" => "required",
    0 => "required",
    1 => "required",
  ]

I also don't think a small potential breaking change warrants not fixing this bug, because it's quite offputting behavior (and the fact that it's broken means it already cannot be used like this, so I don't see what kind of code it would break), and if it's really a problem it can go in a major (which this PR was intended for, but I do agree changing to array_replace_recursive was a breaking change and not the right solution)

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.

3 participants