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

PHP 8.1 support #182

Closed
peterjaap opened this issue Apr 12, 2022 · 13 comments
Closed

PHP 8.1 support #182

peterjaap opened this issue Apr 12, 2022 · 13 comments

Comments

@peterjaap
Copy link

On PHP 8.1, I'm getting this notice;

Deprecated: Optional parameter $name declared before required parameter $fields is implicitly treated as a required parameter in /data/client/magento2/vendor/prismic/php-sdk/src/Prismic/Form.php on line 65

Afaik there's nothing stopping us from changing the constructor to;

diff --git src/Prismic/Form.php src/Prismic/Form.php
index 7de168d..5f5c027 100644
--- src/Prismic/Form.php
+++ src/Prismic/Form.php
@@ -63,12 +63,12 @@ class Form
      * @param array  $fields    the list of Prismic::FieldForm objects that can be used
      */
     private function __construct(
-        ?string $name = null,
-        string  $method,
-        ?string $rel = null,
-        string  $enctype,
-        string  $action,
-        array   $fields
+        string $name = null,
+        string $method,
+        string $rel = null,
+        string $enctype,
+        string $action,
+        array  $fields
     ) {
         $this->name    = $name;
         $this->method  = $method;

@peterjaap
Copy link
Author

Here's another one;

Exception #0 (Exception): Deprecated Functionality: htmlentities(): Passing null to parameter #2 ($flags) of type int is deprecated in /data/client/magento2/vendor/prismic/php-sdk/src/Prismic/Dom/RichText.php on line 154

@c0nst4ntin
Copy link
Collaborator

@peterjaap I ran into similar problems on some client projects and therefore looked into this topic a little bit more. I created a new pull-request (#190) that contains similar changes to #183 but also solves other challenges such as making PHP 8 a minimum requirement and updating the test suite to work with the new version of PHP.

@peterjaap
Copy link
Author

Looks like Prismic has completely forgotten or neglected PHP. Maybe @srenault can help us out by getting Prismic to give the PHP community some love?

@c0nst4ntin
Copy link
Collaborator

@peterjaap I was in contact with some of the developers at Prismic previously. I'll try to address this and see if we can get this repository back to its old glory. Maybe even improve some of the formatting and code quality

@pTinosq
Copy link
Contributor

pTinosq commented Sep 30, 2022

Any updates on this? Really praying for 8.1 support soon 😥

@peterjaap
Copy link
Author

@pTinosq we've given up on buy-in from Prismic. They're a JS crowd, they don't like PHP. You can use our fork instead https://github.com/elgentos/prismicio-php-kit

@pTinosq
Copy link
Contributor

pTinosq commented Sep 30, 2022

@pTinosq we've given up on buy-in from Prismic. They're a JS crowd, they don't like PHP. You can use our fork instead https://github.com/elgentos/prismicio-php-kit

Oh wow that's really annoying. Should I use a specific fork or is the elegentos version sufficient?

@peterjaap
Copy link
Author

@pTinosq the elgentos one is most likely sufficient if you're looking PHP 8.1 compat

@c0nst4ntin
Copy link
Collaborator

c0nst4ntin commented Sep 30, 2022

@pTinosq They are currently taking a closer look at this PR: #190

If I followed along on Twitter correctly @angeloashmore was recently visiting the office and is now on holiday. Let's wait and see what comes from the PR. I trust, that they will give us an update shortly.

As they mentioned in the PR of mine they are simply focussing more on JS but still care about PHP:

In the meantime, this PR isn't being ignored! Before merging, however, we'll need to do some testing of our own.

The PR fully supports PHP 8 and passes all CI Checks implemented on this repository as well.

@pTinosq
Copy link
Contributor

pTinosq commented Sep 30, 2022

Ah that's good. I'll keep my notifications on for the PR. Hopefully Prismic can pull through 🙏🙏

@c0nst4ntin
Copy link
Collaborator

@peterjaap @pTinosq I'm happy to tell you, that this issue was fixed in feat!: support PHP 8 #190

To make use of this change please upgrade to Version 5.2.0

I will proceed to close this issue.

@peterjaap
Copy link
Author

@c0nst4ntin thanks!! 🙌

@pTinosq
Copy link
Contributor

pTinosq commented Nov 8, 2022

@c0nst4ntin This is the news I needed to wake up to. Fantastic stuff 🎉🎉

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