impress-org / givewp

GiveWP - The #1 Donation Plugin for WordPress. Easily accept donations and fundraise using your WordPress website.
https://givewp.com/
GNU General Public License v3.0
345 stars 191 forks source link

Fatal Error: Declaration of Illuminate\Http\Request::get must be compatible with Symfony\Component\HttpFoundation\Request::get #6373

Closed robrecord closed 1 year ago

robrecord commented 2 years ago

User Story

As a developer using Sage (a theme that uses laravel) and PHP 8, I want to install the plugin, but it gives a fatal error

Details

Fatal error: Declaration of Illuminate\Http\Request::get(string $key, $default = null) must be compatible with Symfony\Component\HttpFoundation\Request::get($key, $default = null)

Expected Behavior

Works.

Steps to Reproduce

  1. Install Sage theme https://roots.io/sage/
  2. Install and activate GiveWP plugin
  3. View error

Visuals

Screenshot 2022-04-21 at 13 46 54

Additional Context

This looks like a mismatch of Laravel/Symphony versions

System Information

Details PHP 8 Laravel 9 Sage v10.1.6 Bedrock Trellis

Acceptance Criteria

robrecord commented 2 years ago

I think to fix this it would require the dependency symfony/http-foundation to be updated to ^6.0, as this is what is required by laravel 9 which the sage theme uses.

robrecord commented 2 years ago

Confirmed that requiring "symfony/http-foundation:^5.4" fixes the issue.

Shall I submit a pull request?

jonwaldstein commented 2 years ago

@robrecord unfortunately we can't upgrade that package because we have to support php 5.6 at the moment. I'll discuss with the team to see if there's anything we can do to prevent these types of conflicts.

JasonTheAdams commented 2 years ago

We've been looking at using Strauss within GiveWP to prefix certain vendor namespaces and avoid such conflicts. This is most likely what we'll do.

robrecord commented 2 years ago

Thank you.

I looked at namespacing with composer; for my use case for pretty much need to sort this out so I’ll make a fork with the affected packages namespaced using Strauss or php-scoper. If you like it maybe you could take a pull-request.

robrecord commented 2 years ago

I managed to get it mostly working by upgrading the symphony package and removing faker (required for PHP 8), although the result of that is I get fatal errors when using wp-cli so I need to go down the scoping route. Will let you know how it goes.

robrecord commented 2 years ago

I attempted and failed to get Strauss to work; its latest version v0.11 requires PHP 7, and the version compatible with PHP 5.6 (v0.3) uses a completely different mechanism which I believe would require rewriting a lot of namespace references, which I decided not to do.

My workaround

Instead I present my solution to the issue in the hope that it may help someone in future:

Fix release bundling

In composer.json, edit the second entry of scripts.bundle to rsync -rc --exclude-from=.distignore ./ release/ --delete --delete-excluded - note the added forward slash after the period.

Then, add "release" to a new line at the end of .distignore

(Without those two steps, the release directory gets put into a directory inside itself.)

Update dependency and bundle

Then:

composer config platform.php "7.2.5"
composer require "symfony/http-foundation:^5.4"
composer update --lock --no-dev
npm install && npm run production  # optional
composer bundle
rsync -rc --exclude-from=.distignore release/ ../your-site/wp-content/plugins/give/ --delete --delete-excluded

Hopefully I haven't missed any steps; this allowed the plugin to work for me.

My fork illustrates the changes: https://github.com/robrecord/givewp

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 14 additional days.

arnespremberg commented 1 year ago

Thanks to @robrecord - I successfully installed your fork.

Compatibility with the roots ecosystem is very important in my eyes... At what point does it become more important than backwards compatibility with PHP 5.6? Is there any other chance to get a workaround @jonwaldstein?

jonwaldstein commented 1 year ago

Hey @arnespremberg and @robrecord,

Just an FYI we have since updated our php minimum support to php v7.0.

I just installed the latest version of sage and givewp and did not receive this error. Is this still an issue for you?

arnespremberg commented 1 year ago

hi @jonwaldstein

Yes, for me this is still an issue. It may be related to Bedrock though? Not sure, but happy to provide a testing environment if needed :)

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 14 additional days. Note, if this Issue is reporting a bug, please reach out to our support at https://givewp.com/support. If this is a feature request, please see our feedback board at feedback.givewp.com — that’s the best place to make feature requests, unless you’re providing a PR.