Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Laravel9 support #160

Closed
s-ichikawa opened this issue Feb 14, 2022 · 12 comments
Closed

Laravel9 support #160

s-ichikawa opened this issue Feb 14, 2022 · 12 comments

Comments

@s-ichikawa
Copy link
Owner

I'm working on making support the laravel v9 now.
#159

There was a big change in the mail component from laravel9.
laravel/framework#38481

But I'm not using the laravel recently. so please try the for_laravel9 branch and create an issue if you face some problems.

If there is no critical issue until 2/22/2022, I'll merge into master branch and tag 4.0.0.

@kustomrtr
Copy link

kustomrtr commented Feb 14, 2022

Hey, thanks for the update. I've been testing the for_laravel9 branch and so far it's been working great!

Just a few things I encountered while setting this up, the new readme asks for adding these lines into the app.php under boostrap directory:

$app->configure('mail');
$app->configure('services');
$app->register(Sichikawa\LaravelSendgridDriver\MailServiceProvider::class);

unset($app->availableBindings['mailer']);

image
image

I get the errors above. If I try to run the app, I get:
Fatal error: Uncaught Error: Call to undefined method Illuminate\Foundation\Application::configure() in C:\git\project\bootstrap\app.php:55 Stack trace: #0 C:\git\project\public\index.php(47): require_once() #1 {main} thrown in C:\git\project\bootstrap\app.php on line 55

If I remove the "configure" lines, then I get this

Fatal error: Uncaught ReflectionException: Class "availableBindings" does not exist in C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php:877 Stack trace: #0 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(877): ReflectionClass->__construct('availableBindin...') #1 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(758): Illuminate\Container\Container->build('availableBindin...') #2 C:\git\project\vendor\laravel\framework\src\Illuminate\Foundation\Application.php(855): Illuminate\Container\Container->resolve('availableBindin...', Array, true) #3 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(694): Illuminate\Foundation\Application->resolve('availableBindin...', Array) #4 C:\git\project\vendor\laravel\framework\src\Illuminate\Foundation\Application.php(840): Illuminate\Container\Container->make('availableBindin...', Array) #5 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(1417): Illuminate\Foundation\Application->make('availableBindin...') #6 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(1453): Illuminate\Container\Container->offsetGet('availableBindin...') #7 C:\git\project\bootstrap\app.php(57): Illuminate\Container\Container->__get('availableBindin...') #8 C:\git\project\public\index.php(47): require_once('C:\\git\\govassis...') #9 {main} Next Illuminate\Contracts\Container\BindingResolutionException: Target class [availableBindings] does not exist. in C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php:879 Stack trace: #0 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(758): Illuminate\Container\Container->build('availableBindin...') #1 C:\git\project\vendor\laravel\framework\src\Illuminate\Foundation\Application.php(855): Illuminate\Container\Container->resolve('availableBindin...', Array, true) #2 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(694): Illuminate\Foundation\Application->resolve('availableBindin...', Array) #3 C:\git\project\vendor\laravel\framework\src\Illuminate\Foundation\Application.php(840): Illuminate\Container\Container->make('availableBindin...', Array) #4 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(1417): Illuminate\Foundation\Application->make('availableBindin...') #5 C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php(1453): Illuminate\Container\Container->offsetGet('availableBindin...') #6 C:\git\project\bootstrap\app.php(57): Illuminate\Container\Container->__get('availableBindin...') #7 C:\git\project\public\index.php(47): require_once('C:\\git\\govassis...') #8 {main} thrown in C:\git\project\vendor\laravel\framework\src\Illuminate\Container\Container.php on line 879

If I remove the unset line then all goes OK.

@s-ichikawa
Copy link
Owner Author

@kustomrtr Thank you for your trial and reporting.
I'd like to make sure, that's when you try to install it on Laravel instead of Lumen, right?
These configure are for the installation of Lumen.

@kustomrtr
Copy link

@s-ichikawa omg I feel stupid now, I re-checked the readme and you're in fact right. I was adding those Lumen settings on a Laravel installation, I'm blind hahaha. Thanks again!

@s-ichikawa
Copy link
Owner Author

OK, Thank you for your trying!

@ghost
Copy link

ghost commented Feb 15, 2022

Also going to test this out! I did notice a small issue while upgrading:

We're using the SendgridTransport::SMTP_API_NAME constant, which doesn't exist anymore. Changed it to SendgridTransport::REQUEST_BODY_PARAMETER

@s-ichikawa
Copy link
Owner Author

@rjp-thijs Ahh, yes. I changed that const name.
It might have been a bit rough as a way to change the const name. For the time being, leave SMTP_API_NAME as well and deprecate it.
Thank you!

@ghost
Copy link

ghost commented Feb 21, 2022

Just wanted to report back that we've been using this in production for about a week now and havent noticed any issues :)

@Koozza
Copy link

Koozza commented Feb 21, 2022

@s-ichikawa (FYI @rjp-thijs is my other account ;)) Migtht have cheered a bit too early. It seems that something messes up with our attachments. I'm going to dive into this issue now, but I just wanted to pre-warn you :) PDF and XML attachments get attached as they should, but our CSV's are added as plain text below the mail.

@Koozza
Copy link

Koozza commented Feb 21, 2022

Figured it out. Left a fix at the PR: #159

@s-ichikawa
Copy link
Owner Author

@Koozza Fixed it!
Thank you very much!

@Koozza
Copy link

Koozza commented Feb 23, 2022

I would be fine with releasing this as 1.4 :) Haven't noticed any other bugs!

@s-ichikawa
Copy link
Owner Author

I merged #159 and tagged 4.0.0.
@kustomrtr @rjp-thijs @Koozza
Thank you very much for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants