Running into a method name conflict with the "items" method in ViewModel.php

[nickturrietta]

A suggestion...

It may be beneficial to rename the "items" method within "ViewModel.php" to something a bit more obscure.

I ran into a conflict today where I wanted to use $items within my blade template, but I obviously could not redeclare the "items" method in my ViewModel.

I worked around the issue by making $items a public property on my ViewModel and setting its value within the constructor, but I don't think this would be the best solution.

Thanks for the great package!

[freekmurze]

@brendt could we rename our property to ViewModelItems without it being a breaking change?

[brendt]

It can't be changed without the chance of someone's project breaking, unfortunately.

I'd suggest going ahead with this change, and simply tag it as a new major version. I'd also suggest making this new method private to avoid future problems 😏

@nickturrietta are you up for submitting a PR for this one? Otherwise I'll be happy to fix it for you

[nickturrietta]

Hmmm. But if the new method is made private, then if users wish to change the logic that currently exists in items(), then they would have to modify toArray() as well.

Trying to think of the simplest solution that would still let users override the logic that currently exists in items() (since these are the users for which this would be a breaking change in the first place).

Not sure this is the best, but maybe something like this...

public function toArray() {
  return $this->viewModelItems()->all();
}

private function viewModelItems() {
  return $this->createViewModelItems();
}

protected function createViewModelItems() {
  // the logic currently in items(); encourage users to extend here
}

If you think this is the right approach, I'd be happy to submit a PR. Not sure about the method names though. Will gladly defer to what the team thinks is best.

[brendt]

The items() method was never intended to be overridden though. It was made protected because of our old guidelines. This method is a big part the core of the package, if people want to override it, I'd say this package is not for them.

Even though I doubt anyone is actually overriding it, I believe the right thing to do is add this as a breaking change.

[spatie-bot]

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.