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

Class based after validation rules feature breaks my application #46827

Closed
aedart opened this issue Apr 19, 2023 · 3 comments
Closed

Class based after validation rules feature breaks my application #46827

aedart opened this issue Apr 19, 2023 · 3 comments

Comments

@aedart
Copy link
Contributor

aedart commented Apr 19, 2023

Laravel Version

10.8.0

PHP Version

8.1

Database Driver & Version

n/a

Description

The features added by #46757 sadly breaks my application, due to conflict using the method name "after", in custom FormRequests. I have several custom requests that adds additional "business logic" validation using my own after() method, which seems to have been "high-jacked" for a different purpose, by mentioned feature.

The following code in FormRequest causes PHP "Value of type null is not callable" error (in my situation), because I already am using an "after" method in my custom form requests for something else.

// Inside \Illuminate\Foundation\Http\FormRequest::getValidatorInstance()

// This causes an error (in my application)
if (method_exists($this, 'after')) {
    $validator->after($this->container->call(
        $this->after(...),
        ['validator' => $validator]
    ));
}

I'm at odds here... The feature is nice, but changing my application will cost me some time! Is there any chance that you can revert the feature and possible adapt it to handle situations when the form request might already use an after()method for something else? E.g. perform an instanceof check against an appropriate interface that ensures the feature's desired method name and return type?

Steps To Reproduce

The following is a typical example of a custom form requests in my application. I use a method named after() to perform whatever additional validation I might require. Furthermore, I do not return anything from the method.

class CreateItem extends FormRequest
{
    public function withValidator(Validator $validator)
    {
        $validator->after([$this, 'after']);
    }

    public function after(Validator $validator): void
    {
        // Some custom "business logic" validation, across fields, additional queries, ...etc
    }

   // ...remaining not shown...
}

The above code will cause the new feature to fail, because it expects the after method to return an array (as far as I understood the new feature).

@driesvints
Copy link
Member

Hi @aedart. I'm very sorry this broke your application. However, I don't feel we can qualify this as a breaking change. Over time, we've run into similar situations where we introduce new methods that conflict with methods written by people themselves. SemVer allows us to introduce new backwards compatible code into the public API: https://semver.org/#spec-item-7

Unfortunately it's impossible to take into account every situation where a developer may have introduced a similar method. This would mean we'd never ever have the ability to add methods into the public API without releasing a new major version (which is once a year). I hope you can understand our position. Again sorry you got caught with this.

@aedart
Copy link
Contributor Author

aedart commented Apr 21, 2023

Hi @driesvints
Sorry for my rather late reaction - it's okay. Things like this sometimes happen. I managed to find a solution for my problem. Also, I apologize if the references/issue-links has drawn "negative" attention to this small matter. Due to my job, I sometimes need to be formal and ensure appropriate traceability.

@stevelacey
Copy link
Contributor

stevelacey commented Dec 19, 2023

If anyone else finds themselves here: Spark Classic also breaks at this point when running on Laravel 10.

Rename after in Laravel\Spark\Http\Requests\Auth\RegisterRequest and it's one call in registerValidator to something else and you should be good to go.

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

No branches or pull requests

3 participants