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

Migrated references from Leafo to ScssPhp cause Fatal error - Version 2.0.0 #157

Closed
kinsky-org opened this issue Feb 8, 2021 · 6 comments
Labels

Comments

@kinsky-org
Copy link

kinsky-org commented Feb 8, 2021

Hi there! Just spotted an issue after the upgrade to WP-SCSS@v2.0.0.
#156 and changing the vendor prefixes from "Leafo" in option keys (qualified class names) in commit 9b7bc1b5d8250580be585cdb1525239ee7ecca1d can cause following consequences:

  1. user already has the old vendor name saved in database as a part of a class name;
  2. user upgrades WP-SCSS plugin to the latest version;
  3. things stop working after reload because the get_option call is still getting that old vendor name as a part of a qualified class name but class doesn't exist anymore.
PHP Fatal error:  Uncaught Error: Class 'Leafo\ScssPhp\Formatter\Crunched' not found in /www/doc/www.example.cz/www/wp-content/plugins/wp-scss_invalid/scssphp/src/Compiler.php:243
Stack trace:
#0 /www/doc/www.example.cz/www/wp-content/plugins/wp-scss_invalid/class/class-wp-scss.php(79): ScssPhp\ScssPhp\Compiler->compile('@import "common...', '/www/doc/www.ex...')
#1 /www/doc/www.example.cz/www/wp-content/plugins/wp-scss_invalid/class/class-wp-scss.php(112): compiler('/www/doc/www.ex...', '/www/doc/www.ex...', Object(Wp_Scss))
#2 /www/doc/www.example.cz/www/wp-content/plugins/wp-scss_invalid/wp-scss.php(196): Wp_Scss->compile()
#3 /www/doc/www.example.cz/www/wp-content/plugins/wp-scss_invalid/wp-scss.php(180): wp_scss_compile()
#4 /www/doc/www.example.cz/www/wp-includes/class-wp-hook.php(287): wp_scss_needs_compiling('')
#5 /www/doc/www.example.cz/www/wp-includes/class-wp-hook.php(311): WP_Hook->apply_filters(NULL, Array)
#6 /www/doc/www.example.cz/www/wp-includes/plugin.php(484): WP_Hook->do_action(Array)
#7 /www/doc/www.example.cz/www in /www/doc/www.example.cz/www/wp-content/plugins/wp-scss_invalid/scssphp/src/Compiler.php on line 243

If anyone needs it hotfixed it might be done ie. by changing the old option_value in wp_options in the database (option_name='wpscss_options' and option_value LIKE '%Leafo\ScssPhp\Formatter%' => option_value should be changed to 'ScssPhp\ScssPhp\Formatter') or using a filter hook "option_wpscss_options" etc.

Thanks for this plugin and for all your work. I'm sorry I can't help more at the moment, just have to deal with something else too.

@vanling
Copy link

vanling commented Feb 8, 2021

Thanks @kinsky-org, you saved my 🍑 !

I have the 2.0 version installed at 10 sites and only one was broken.

Maybe the broken value helps.

'a:6:{s:8:\"scss_dir\";s:13:\"/assets/scss/\";s:7:\"css_dir\";s:12:\"/assets/css/\";s:17:\"compiling_options\";s:34:\"Leafo\\ScssPhp\\Formatter\\Compressed\";s:17:\"sourcemap_options\";s:17:\"SOURCE_MAP_INLINE\";s:6:\"errors\";s:4:\"show\";s:7:\"enqueue\";s:1:\"1\";}

@shadoath shadoath added the bug label Feb 8, 2021
@shadoath
Copy link
Collaborator

shadoath commented Feb 8, 2021

@kinsky-org Thank you for pointing this out and providing a hotfix. Now I see how the plugin is still falling back to a DB value that may be the Leafo not ScssPhp.

@shadoath shadoath pinned this issue Feb 8, 2021
@shadoath
Copy link
Collaborator

shadoath commented Feb 8, 2021

@kinsky-org here is what I'm thinking for a quick fix. It will read correctly but allows Leafo to stay in DB. so not ideal but would stop this error taking down a website.

@shadoath
Copy link
Collaborator

shadoath commented Feb 8, 2021

Version 2.0.1 has been updated to WP plugins. Leaving open for now in case others are on 2.0.0 and need a quick fix before updating.

@shadoath shadoath changed the title Migrated references from Leafo to ScssPhp cause Fatal error Migrated references from Leafo to ScssPhp cause Fatal error - Version 2.0.0 Feb 8, 2021
@kinsky-org
Copy link
Author

kinsky-org commented Feb 8, 2021

@kinsky-org here is what I'm thinking for a quick fix. It will read correctly but allows Leafo to stay in DB. so not ideal but would stop this error taking down a website.

@shadoath Yep, that's exactly what I would do as a hotfix if my Adminer wasn't already connected and going through the options table (I was updating a client's website after years of no maintenance and many of those 10+ plugins were erroring much worse) which allowed me to fix it in almost no time with a simple SQL query.

It also makes me remember the times when we were working on one hobby project (online multiplayer game) in C# and we needed to save every bit of information like twice per hour and load it every time the server went down and got running again. That brought a lot of errors like this one because people were always adding and removing variables like crazy and sometimes even reverting code to previous version so we needed to solve that for almost every single class. We used a version number for every class (same as you here with that constant) which was loading something - so we added some comparisons to the save and load routines - loading the version number first to choose which other variables should be assigned, modified and removed/deprecated (if version >= 15 load the new variable but skip those that we just removed).

I wonder if such solution would be appropriate, but I think it would be best if it's implemented on "higher level" with some support in WP core. I guess someone was already thinking about this problem as I see in add_option:

    /*
     * Until a proper _deprecated_option() function can be introduced,
     * redirect requests to deprecated keys to the new, correct ones.
     */
    $deprecated_keys = array(
        'blacklist_keys'    => 'disallowed_keys',
        'comment_whitelist' => 'comment_previously_approved',
    );

If I'm not mistaken it might be totally possible - if there was just one additional filter hook or something like that - to deprecate or "redirect" options more easily and maybe even remove the old clutter from database.
But right now it seems like you probably even need to add your own update routine if you need to do anything before/after the plugin update as the activation hook is not reliable for that case.

Anyway - thanks a lot for such a fast reaction with even an update to WP plugins - probably saving some people from headaches!
I also really appreciate the mention in readme (there's just a missing letter in my nickname, but that's not important ;) ), although it was no big deal and I'm more like grateful for this plugin that I use for years on some websites - mostly if there's no gulp/webpack to handle that, your plugin is the easy option that works great "out of the box".
PS: And sorry for the long message, I sometimes tend to think out loud. .)

@shadoath
Copy link
Collaborator

shadoath commented Feb 9, 2021

@kinsky-org good links to review, thanks for listing them out here. My apologies for the typo in your name (was a bit early this morning when I was making the update (now corrected). I like the idea for the update routine the best. Something to dig into more later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants