Fix facade diskName

[SanderMuller]

1️⃣ Why should it be added? What are the benefits of this change?
Currently the facade has an incorrect PHPDoc stating that the store method of Excel.php accepts the $disk argument while it actually accepts the $diskName argument. This causes confusing behaviour, for example incorrect IDE autocompletion or PHPStan errors

2️⃣ Does it contain multiple, unrelated changes? Please separate the PRs out.
No

3️⃣ Does it include tests, if possible?
No

4️⃣ Any drawbacks? Possible breaking changes?
No breaking change.

A drawback is that other methods in Excel.php use $disk instead of $diskName, so using $disk instead of $diskName would be more consistent, but that would be a breaking change. Using $diskName everywhere would make the most sense, since we're not receiving an instance of a Disk, but that would be an even bigger breaking change I think.

5️⃣ Mark the following tasks as done:

• Checked the codebase to ensure that your feature doesn't already exist.
• Take note of the contributing guidelines.
• Checked the pull requests to ensure that another person hasn't already submitted a fix.
• Added tests to ensure against regression.

6️⃣ Thanks for contributing! 🙌
yw :)

[patrickbrouwers]

Does it cause issues that the ExcelFake store() method refers to disk instead of diskName?

[SanderMuller]

Does it cause issues that the ExcelFake store() method refers to disk instead of diskName?

It causes PHPStan to think the code is incorrect:

  36     Unknown parameter $diskName in call to static method      
         Maatwebsite\Excel\Facades\Excel::store().  

[joelharkes]

If you fix it everywhere it would be consistent

[joelharkes]

ther are some more places to fix this:

[patrickbrouwers]

However changing it in other places like Exportable would make it a breaking change, as someone doing the following

(new MyExport)->store('path', disk: 'public')

would get an error if we can it to diskname.

I think for now it would probably be best to just update the facade docblock and leave the rest until a major release with upgrade notes.

[SanderMuller]

ther are some more places to fix this:

Except that in other places in the source code (non Facade) $disk is used, store is the exception

[patrickbrouwers]

What if we leave everything as is, and just add a fallback option to Excel@store, so disk: 'name' would work as well via the facade.

public function store($export, string $filePath, string $diskName = null, string $writerType = null, $diskOptions = [], string $disk =null)

That way no breaking change, but ensuring consistency with the facade and the other locations. (Honestly absolutely no idea why it isn't consistent, it of course didn't matter before named params)

[SanderMuller]

Sounds fine to me, then we can drop one of the two in a major version

[patrickbrouwers]

Yes exactly, can you update your PR? Can you add a comment to that docblock that it serves as a fallback for named params consistency?

[stale]

This bug report has been automatically closed because it has not had recent activity. If this is still an active bug, please comment to reopen. Thank you for your contributions.

[patrickbrouwers]

Replaced by 81c921d