[5.5] Changing events property to avoid any conflicts with relations

[arcanedev-maroc]

Related to :

• Issue: [5.4] Call to a member function isEmpty() on array #17959
• PR: [5.4] Changing events property to avoid any conflicts with relations #17960

[fernandobandeira]

Maybe we should just document this as a reserved word somewhere since it won't be the only property name that may cause issues with relationship names.

Also if we were changing this personally i'd prefer to use something like _events since its shorter, this way we could maybe define and use a prefix to avoid these kind of conflicts in the future, still IMHO it's not worth changing this since it'll break packages that rely on this variable name...

[arcanedev-maroc]

Hi @fernandobandeira,

It's not going to be prefixed with underscore because it's against the PSR-2 coding style guide.

And since this change is a breaking compatibilty, i put it in the master branch instead.

I like the events as relationship name because it make sense when you work with calendars.

And $observableEvents property is also a good name (self explanatory) to solve this conflict.

[fernandobandeira]

According to PSR-2

Property names SHOULD NOT be prefixed with a single underscore to indicate protected or private visibility.

We are not prefixing it to indicate protected or private visibility we would be prefixing to avoid a conflict, also we can use another prefix if undescore is a bad thing, PSR-2 doesn't seems to restrict this. ( It's up to your interpretation )

Honestly docummenting it would be the best thing since you wouldn't create a BC and it seems like an edge case, just giving my opinion here... 😄

IK it won't look as nice as events but you can easilly rename it to something else, now think if a package is using this directly they'll have to make a check for versions >= 5.5 and use another variable just for this change that isn't really necessary...

[arcanedev-maroc]

If the events name was reserved for the model observers (HasEvents Trait) and may have conflicts when we using it as table (events) / model (Event) / relationship (events()) => name convention.

So it should warn us in the documentation before it's late 😢

[bryannielsen]

I'd love to know what the official decision on this is. I have a few projects that refer to calendar events as well so I don't really want to rename the models/relationships if possible.

[taylorotwell]

Changed this to $dispatchesEvents.

[arcanedev-maroc]

Great job @taylorotwell 👍