Skip to content

Conversation

@Neol3108
Copy link
Contributor

@Neol3108 Neol3108 commented Apr 2, 2024

Idea behind this PR

This PR offers an alternative to defining the "magic" deleteWhenMissingModels. I often struggle to remember
the name of this property. I either look through the framework code or look up documentation. I believe
attributes provide a better way to define these side effects. And IDEs can autocomplete them more easily.

Before:

<?php

namespace Acme;

class Job
{
    public $deleteWhenMissingModels = true;
}

After:

<?php

namespace Acme;

use Illuminate\Queue\Attributes\DeleteWhenMissingModels;

#[DeleteWhenMissingModels]
class Job
{
    //
}

Breaking changes

None come to mind.

Personal suggestion

I personally feel the deleteWhenMissingModels property should be removed in the future in favor of this
attribute. I don't think it makes sense to maintain two approaches to do this. I guess this would look like
a deprecation in 12.x and removal in 13.x? As there is no way to deprecate something that isn't defined
in the framework itself but only referenced, I think a note in the upgrade guide would have to be enough.
A slight breaking change when switching over from the property to the attribute would be that attributes
are not inherited along with classes (jobs) themselves. So if you extend a job with the deleteWhenMissingModels
property and that gets replaced by the attribute it would break the behaviour of the "child job".
That might be worth noting in the docs

Docs

If this PR gets merged I think it should also be reflected in the documentation. I will create a PR there as well.

@taylorotwell taylorotwell merged commit 96593e2 into laravel:11.x Apr 3, 2024
@taylorotwell
Copy link
Member

Dig it.

@Neol3108 Neol3108 deleted the delete-model-attribute branch April 4, 2024 07:28
@siarheipashkevich
Copy link
Contributor

@Neol3108 what about the documentation for this?

@Neol3108
Copy link
Contributor Author

@siarheipashkevich PR was closed by Taylor laravel/docs#9650

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