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

Enhanced Router Compatibility (Including Livewire Support) And Filters For Early Service Container Overrides #291

Merged
merged 38 commits into from Jan 5, 2024

Conversation

broskees
Copy link
Contributor

@broskees broskees commented Jun 12, 2023

This PR is primarily focused on enhancing router compatibility in general, which also has the side benefit of Livewire being natively supported.

The key changes include:

  1. Allowing kernels to be overridden via filters. This change enhances the flexibility of the application by allowing the use of custom kernels. Addresses Document how to override Acorn kernel / boot docs#507. Unfortunately I can't do class replacement like mentioned here because of autoloaders if Acorn is required in Bedrock or Radicle, basically the same issue described here.
  2. Fixing Acorn router load issue when WP_CLI is active: This addresses an issue where the Acorn router was not being loaded when WP_CLI was active.
  3. Bootloader changes: These changes improve router compatibility and address various bugs. This includes updates to the boot method and bootHttp method in the Application.php file, which notably change the handling of requests and responses. Basically, the router now has a default "route" for WordPress requests, so all requests use the router. This more closely mirrors Laravel's native functionality and thus offers better compatibility with laravel packages (like Livewire).

You can test these changes with a Livewire setup in an Acorn site. Please follow the steps below:

  1. Install Livewire and its dependencies: composer require illuminate/testing illuminate/queue illuminate/encryption livewire/livewire
  2. Set an APP_KEY to a randomized 32-character string in your .env file.
  3. Add Livewire scripts to your Sage theme using the provided PHP snippet:
collect([
    'wp_head' => '@livewireStyles',
    'wp_footer' => '@livewireScripts',
])->each(fn ($directive, $hook) =>
    add_action($hook, function () use ($directive) {
        echo Blade::render($directive);
    })
);
  1. Create a test component with Livewire CLI: wp acorn make:livewire Counter
  2. Edit the component to test

@broskees broskees changed the title Fix for Router Not Loading Http Kernel When Using WP_CLI Enhanced Router Compatibility (Including Livewire Support) And Filters For Early Service Container Overrides Jun 14, 2023
Copy link
Sponsor Member

@QWp6t QWp6t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey thanks for the PR! 🙏

This looks fantastic, and it's something I had been meaning to tackle. This is a great start for allowing users to override our classes. I'm not entirely sold on the use of WordPress filters here, but I'm willing to give it a shot for now.

But I do see room for some refactors to make it a little more readable for future me. 😅

Let me know if you want me to jump in and handle these edits or if you wanna tackle it.

Thanks again for the PR. I appreciate you digging into this.

src/Roots/Acorn/Bootloader.php Outdated Show resolved Hide resolved
src/Roots/Acorn/Bootloader.php Outdated Show resolved Hide resolved
Copy link
Sponsor Member

@QWp6t QWp6t left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great!

I won't have time to properly test this until later in the week, but it all looks good to me at the moment.

edit: except for the lint errors 😅 (i can fix that before i merge)

@broskees
Copy link
Contributor Author

broskees commented Aug 1, 2023

@QWp6t Anything I can do to help get this merged?

@adriansalvatori
Copy link

I'm eager to test this! Same as broskees, Any way I can help to get this merged?

@Log1x
Copy link
Sponsor Member

Log1x commented Sep 8, 2023

Messed around with this a bit – are you able to save posts in the editor with custom routes enabled? I get an error due to the PUT request to wp-json not being permitted by the Laravel router.

@adriansalvatori
Copy link

I'm playing with this and nope, I'm also not being able to save posts in the editor. Had to disable gutenberg to keep playing haha.

@broskees
Copy link
Contributor Author

Messed around with this a bit – are you able to save posts in the editor with custom routes enabled? I get an error due to the PUT request to wp-json not being permitted by the Laravel router.

@Log1x & @adriansalvatori I'll check that out now, and also resolve conflicts.

@broskees
Copy link
Contributor Author

@Log1x & @adriansalvatori - Sorted that out. Thank you and good catch.

@tgeorgel
Copy link

Hey @broskees, thanks for your work on this :)

Unfortunately, on Radicle stack the Illuminate\Support\Facades\Request::create() method is giving back a Symfony\Component\HttpFoundation\Request instead of a Illuminate\Http\Request class when called from Livewire sources.

This cause multiple error with Livewire bootstrapping, when issuing any refresh request, as it expect some methods from Illuminate request.

screenshot 2023-11-11 à 11 39 20

Strange thing, app('request') — or the facade itself — called from an application provider is successfully returning an Illuminate Request, and not a Symfony 😅.

Steps applied :

  • Apply the PR on Acorn main branch
  • Use this new fork as Acorn replacement
  • Follow the steps described in your PR description

@sammyaxe
Copy link

@tgeorgel have you found a solution? I'm not getting the error on Radicle, but while livewire component renders fine, I can't seem to interact with it.

@tgeorgel
Copy link

@tgeorgel have you found a solution? I'm not getting the error on Radicle, but while livewire component renders fine, I can't seem to interact with it.

Hey @sammyaxe, I have the exact same situation as you, the render is working fine, it's on interaction with the component that the error is triggered (when the Livewire update route is called).

I didn't find any solution as for now.

@broskees
Copy link
Contributor Author

broskees commented Nov 17, 2023

Hey @broskees, thanks for your work on this :)

Unfortunately, on Radicle stack the Illuminate\Support\Facades\Request::create() method is giving back a Symfony\Component\HttpFoundation\Request instead of a Illuminate\Http\Request class when called from Livewire sources.

This cause multiple error with Livewire bootstrapping, when issuing any refresh request, as it expect some methods from Illuminate request.

screenshot 2023-11-11 à 11 39 20

Strange thing, app('request') — or the facade itself — called from an application provider is successfully returning an Illuminate Request, and not a Symfony 😅.

Steps applied :

* Apply the PR on Acorn main branch

* Use this new fork as Acorn replacement

* Follow the steps described in your PR description

@tgeorgel Thanks for pointing this out. I'll take a look soon and see if I can find a solution.

@Log1x Log1x self-requested a review January 4, 2024 07:36
@Log1x Log1x self-assigned this Jan 4, 2024
@Log1x Log1x added the enhancement New feature or request label Jan 4, 2024
@Log1x
Copy link
Sponsor Member

Log1x commented Jan 4, 2024

Good work. I merged main (v4) in, cleaned a few things up, and added illuminate/encryption.

This PR is working nicely in my tests with a simple counter component in Livewire 3. We will have to look into implementing key:generate – right now it insists on creating .env in the base path (e.g. Sage 10) instead of falling back to Bedrock. Otherwise, adding a proper APP_KEY to Bedrock's .env is working.

@QWp6t I'm not sure why a test is failing. When I test on my own with trigger_error('This is a warning', E_USER_WARNING); it seems to properly be throwing an ErrorException in Ignition on my end.

After that test is fixed, this can get merged in and we can continue polishing up things as needed in another PR as well as iron out Radicle support (if still necessary).

EDIT: wp acorn key:generate is implemented 👍

@QWp6t QWp6t merged commit e89d969 into roots:main Jan 5, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants