driesvints / vat-calculator

Handle all the hard stuff related to EU MOSS tax/vat regulations, the way it should be.
MIT License
1.2k stars 88 forks source link

Problem with Laravel and VatCalculator autoloading #88

Closed dvigueras closed 3 years ago

dvigueras commented 5 years ago

Hi,

there is an issue with the way that VatCalculatorServiceProvider binds the VatCalculator dependency in Laravel.

This is the registerVatCalculator method:

    /**
     * Register the application bindings.
     *
     * @return void
     */
    protected function registerVatCalculator()
    {
        $this->app->bind('vatcalculator', '\Mpociot\VatCalculator\VatCalculator');

        $this->app->bind('\Mpociot\VatCalculator\VatCalculator', function ($app) {
            $config = $app->make('Illuminate\Contracts\Config\Repository');

            return new \Mpociot\VatCalculator\VatCalculator($config);
        });
    }

Instead of using the special PHP VatCalculator::class constant which resolves to 'Mpociot\VatCalculator\VatCalculator' the method is using: '\Mpociot\VatCalculator\VatCalculator' (note the leading slash)

In my vat_calculator.php config file I have the following config: 'business_country_code' => 'ES',

Which gets applied when creating a new instance of VatCalculator only if VatCalculator is loaded using the Laravel IoC Container.

The problem is that if anywhere in my code I declare VatCalculator as a dependency, Laravel (5.5 at least) doesn't use the binding provided by VatCalculatorServiceProvider, and creates the class with a $config = null.

In php artisan tinker:

>>> app('Mpociot\VatCalculator\VatCalculator')->getTaxRateForLocation('ES', null, true);
=> 0
>>> app('\Mpociot\VatCalculator\VatCalculator')->getTaxRateForLocation('ES', null, true);
=> 0.21

The solution would be to use the name without the leading slash in the registerVatCalculator method of the VatCalculatorServiceProvider (The best would be to use the ::class constant).

I could send you a PR if you are willing to accept it :)

dvigueras commented 5 years ago

I've just discovered there is a pending PR #85 to be merged. That would solve the issue @mpociot

dvigueras commented 5 years ago

While the pending PR gets merged I fixed it creating a new temporary ServiceProvider in Laravel:

class FixVatCalculatorServiceProvider extends ServiceProvider
{
    public function register()
    {
        $this->app->bind(VatCalculator::class, function ($app) {
            return $app->make('\Mpociot\VatCalculator\VatCalculator');
        });
    }
}
driesvints commented 3 years ago

This seems to be merged and released already. Thanks

dvigueras commented 3 years ago

Yes, it was merged on 2 Jun 2020, thanks.