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

Registering customLabel macro once Form resolved #725

Merged
merged 2 commits into from
Apr 4, 2024

Conversation

sangnguyenplus
Copy link
Contributor

No description provided.

@rudiedirkx
Copy link
Collaborator

Why? Service provider order? That has never been a problem for me. Are your SP ordered differently/manually?

@rudiedirkx
Copy link
Collaborator

You probably mean afterResolving(static::FORM_ABSTRACT, (the form service), not the packakge Form class.

@sangnguyenplus
Copy link
Contributor Author

@rudiedirkx Sorry, I forgot to explain it.

I have changed to use afterResolving(static::FORM_ABSTRACT, (the form service)

Actually, you still can use it, and it won't cause any bugs, I have used your package many years and it is still working fine.

But today, I checked this code when trying to replace laravelcollective/html package (it's deprecated already).

laravelcollective/html/src/HtmlServiceProvider.php:59

image

When using form service, it will initialize session, so if you put that code directly in boot() method without wrapping it in afterResolving(static::FORM_ABSTRACT), it will initialize session in boot method, it's not correct app lifecycle. Many HTTP requests don't need session, so session shouldn't be resolved in boot (the same with artisan commands).

With this change, session won't be started in boot() method of service provider.

@rudiedirkx
Copy link
Collaborator

I require rdx/laravelcollective-html in my projects, which composer-replaces laravelcollective/html for compatibility. This package still requires laravelcollective/html to not force a replacement. Such a shame that they quit like this =( What a mess it is now.

How are you replacing laravelcollective/html?

@rudiedirkx rudiedirkx merged commit 62b0a63 into kristijanhusak:master Apr 4, 2024
4 checks passed
@rudiedirkx
Copy link
Collaborator

If you're feeling extra generous, you could look at the tests for Laravel 11 support #721 😄 I don't understand phpunit...

@sangnguyenplus sangnguyenplus deleted the patch-1 branch April 5, 2024 02:47
@sangnguyenplus
Copy link
Contributor Author

@rudiedirkx It just has a few classes https://github.com/LaravelCollective/html/tree/6.x/src. I have copied it into our core project and maintain it.

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

Successfully merging this pull request may close these issues.

2 participants