Moving to PHP 8 without pain for PHP 7 users #716

Activity

1. leonstafford

   added

   investigate

   question

   on Dec 4, 2020
2. leonstafford commented on Dec 5, 2020

   

   leonstafford

   on Dec 5, 2020

   ContributorAuthor

   More actions

   develop branch now looks PHP8 friendly, with cURL requests replaced by Guzzle. needs extensive testing, but so far builds and completes export process
3. john-shaffer commented on Dec 7, 2020

   

   john-shaffer

   on Dec 7, 2020

   Last edited by john-shaffer

   Contributor

   More actions

   What is in PHP 8 that's such an advantage?

   I like the idea of using libs like Guzzle in theory. In practice, I've had broken WPs due to having two plugins with slightly different versions of Guzzle. As in, their dependency specs were exactly equivalent but composer install was run at different times. To avoid that, you'll have to force a specific version of Guzzle in wp2static deps as well as any addons that depend on Guzzle indirectly, such as the S3 addon. And stick to that version for a long time. But we can get broken sites if any other user-installed plugin uses a different version of Guzzle. I don't think there is any way to do dependencies properly in WP.
4. leonstafford commented on Dec 7, 2020

   

   leonstafford

   on Dec 7, 2020

   ContributorAuthor

   More actions

   It's probably just some bright and shininess for me, but some better typing/reflection support. So, I won't be abandoning 7 support anytime soon, just want to get started on 8.

   I'll look into the Guzzle issue, I thought Composer namespacing solved some of that, but seems not. I think there's a way to force it under our own namespace, else, I can do some rewriting into a new namespace to solve that....

   👍React with 👍1Reacted by John Shaffer
5. leonstafford commented on Dec 7, 2020

   

   leonstafford

   on Dec 7, 2020

   Last edited by leonstafford

   ContributorAuthor

   More actions

   

   Sweet, as above, there are things out there to make this easier now, no suprise, from the great minds @ Delicious Brains

   • and another
   • yet another
6. leonstafford commented on Dec 7, 2020

   

   leonstafford

   on Dec 7, 2020

   ContributorAuthor

   More actions

   Reading through this was a useful refresh on the issue.

   I'll add some more thoughts specific to WP2Static.

   This is core's current prod depenencies:

           "php": ">=7.3",                                                         
           "wa72/url": "^0.7.1",                                                   
           "guzzlehttp/guzzle": "^7.2.0",                                          
           "vipnytt/sitemapparser": "^1.1.3" 
   

   I'm quite happy to keep Guzzle in, as we're doing so much requesty stuff and having rewritten it a few times to try cURL/Guzzle/wp_remote's, I'd rather commit to Guzzle for less lines of own code and more confidence.

   vipnytt/sitemapparser is less than 500 LoC really and came in via a contributed new feature. We can keep the great feature, but bring the functionality into it's own WP2Static class.

   wasinger/url is scarily out of date and only there because I'm afraid what happens if I took it out today. I think that was more useful in times of supporting relative/offline URLs in the export. Likely more work than bringing in house the Sitemaps functionality, but a good opportunity to flesh out our own URL class which currently provides interface to wa/url:

       public function __construct( string $url, string $parent_page_url = null ) { 
           $url = new \Wa72\Url\Url( $url );                                          
                                                                                      
           if ( $parent_page_url ) {                                                  
               $this->parent_page_url = new \Wa72\Url\Url( $parent_page_url );        
               $this->url = $url->makeAbsolute( $this->parent_page_url );             
           } else {                                                                   
               // test absolute URL                                                   
               if ( ! $url->getHost() ) {                                             
                   throw new WP2StaticException(                                      
                       'Trying to create unsupported URL'                             
                   );                                                                 
               }                                                                      
                                                                                      
               $this->url = $url;                                                     
           }                                                                          
       }                                                                              
                                                                                      
       /**                                                                            
        * Return the URL as a string                                                  
        */                                                                            
       public function get() : string {                                               
           return $this->url;                                                         
       }                                                                              
                                                                                      
       /**                                                                            
        * Rewrite host and scheme to destination URL's                                
        *  - mutates URL object in place                                              
        *                                                                             
        * @param string $destination_url URL rewrite rules                            
        */                                                                            
       public function rewriteHostAndProtocol( string $destination_url ) : void {   
           $destination_url = new \Wa72\Url\Url( $destination_url );                  
                                                                                      
           $this->url->setHost( $destination_url->getHost() );                        
           $this->url->setScheme( $destination_url->getScheme() );                    
       }

   So, my leaning today is to:

   • remove the small deps which don't spark joy and eliminate any future problems from them (like the sitemaps one not being able to be pulled in for PHP8 envs, meaning WP2Static can't, either, unless we fork their lib and adjust composer.json requirements)
   • maintain own fork of major libs, like Guzzle and AWS SDK, with a simple script to change main namespaces, like Guzzle to WP2StaticGuzzle

   It sounds like a lot of effort, but shouldn't be - once the script to rename the namespace is written, it behaves like any other dependency with custom VCS in our composer.json. I doubt anyone who's using WP2Static really cares what version of Guzzle we're using or what namespace it's under, as long as it's not conflicting with another they have. It shouldn't affect anyone else's build processes, if they're using WP2Static or friends via composer vs zips.

   At most, it should need a comment or more descriptive name for the wrapped Guzzle library to inform developers why/what is is. like

   // our own namespaced fork of Guzzle 
   use WP2StaticGuzzle\Client;

   or

   use WP2StaticsSafelyNamespacedGuzzle\Client;
7. leonstafford

   closed this as completedon Feb 27, 2021
8. elementor

   locked and limited conversation to collaborators on Feb 27, 2021