Use contracts instead of facades

[niklasravnsborg]

Throughout the project I used Laravel facades: https://github.com/niklasravnsborg/laravel-pdf/blob/913fa50ce9f895a0e08faaf5324cc9aa7826a742/src/LaravelPdf/PdfWrapper.php#L5-L6

It might be better to use contracts instead of facades in order to provide a cleaner abstraction and to be less dependent on the service container: https://laravel.com/docs/5.7/contracts

[erfansahaf]

@niklasravnsborg That is a good idea but there is not that much difference between Facades and Contracts. If you use Contract, the class would need an implementation of that Contract and then, Service Container will inject one to the class. So in this case, we still depend on Service Container, as you know. The only benefit would be more cleanliness which is nice but not necessary.

[niklasravnsborg]

@erfansahaf Right. I just stumbled on this in the Laravel facade documentation:

When building a third-party package that interacts with Laravel, it's better to inject Laravel contracts instead of using facades. Since packages are built outside of Laravel itself, you will not have access to Laravel's facade testing helpers.

https://laravel.com/docs/5.7/facades#when-to-use-facades

Thats why I took a note here :)

[colorninja]

Why do you wan't to make it less dependent on the service container? As the service container is most likely staying forever I think it's might be a good idea to use it to it's advantages and implement the wrapper using dependency injection.

It makes sense if you are writing a library for a framework to use the framework features to some extent (eg. Service container).

It would be something like:

<?php

namespace niklasravnsborg\LaravelPdf;

use Illuminate\Filesystem\Filesystem;
use Illuminate\View\Factory;

class PdfWrapper
{

    /**
     * Laravel's blade template renderer
     * @var Factory
     */
    private $viewFactory;

    /**
     * Lavavel file manager
     * @var Filesystem
     */
    private $filesystem;

    public function __construct( Factory $viewFactory, Filesystem $filesystem )
    {
        $this->viewFactory = $viewFactory;
        $this->filesystem = $filesystem;
    }

    /**
     * Load a HTML string
     * @param string $html
     * @return Pdf
     */
    public function loadHTML( $html, $config = [] )
    {
        return new Pdf($html, $config);
    }

    /**
     * Load a HTML file
     * @param string $file
     * @return Pdf
     */
    public function loadFile( $file, $config = [] )
    {
        $html = $this->filesystem->get($file);

        return $this->loadHTML($html, $config);
    }

    /**
     * Load a View and convert to HTML
     * @param string $view
     * @param array $data
     * @param array $mergeData
     * @return Pdf
     */
    public function loadView( $view, $data = [], $mergeData = [], $config = [] )
    {
        $html = $this->viewFactory->make($view, $data, $mergeData)->render();

        return $this->loadHTML($html, $config);
    }

}

And then the binding could be:

$this->app->singleton('mpdf.wrapper', PdfWrapper::class);

Another question would be if that works on older versions of Laravel. I am using PHP 7.4.4 and Laravel 7.X and it works for me. I'm writing this because people with strict PHP Notice/Warnings check and you haven't declared the facades to disallow their usage, you cannot use this library