Spyc Yaml encoder emits PHP 8.1 deprecation warnings

[neildaniels]

Description

The Spyc Yaml encoder emits frequent deprecation warnings under PHP 8.1.

Example:

trim(): Passing null to parameter #1 ($string) of type string is deprecated

line 353 of /var/www/html/vendor/mustangostang/spyc/Spyc.php

You can disable deprecation warnings completely (with your .ini file or error_reporting()), but that silences all deprecation warnings that might be in user-code.

Expected behavior

I'm not sure what the best approach here is.

Spyc seems kind of abandoned. This package will be frustrating to use if it's not updated by the time these warnings become errors.

Maybe allowing the Yaml class to use different Yaml parser/encoder would be ideal? For example, would be nice to be able to do something in config to use PECL's Yaml package.

Screenshots

To reproduce

Yaml::encode(['test' => null]);

Your setup

Kirby Version

3.6.1.1

Additional context

[distantnative]

We have tried to move away from Spyc a few times. The issue always is that YAML parsers aren't all very consistent but differ quite a bit in their behavior. And that would be a very big breaking change - which is why we never proceeded so far.

[afbora]

I think it's time to switch to Symfony YAML component. We gave up because it involved breaking changes at the time, but I guess we should do in next major version.

Related PR https://github.com/getkirby/kirby/pull/2453

[distantnative]

In our unit tests alone we'd have one breaking change already: https://github.com/getkirby/kirby/commit/8207464882d8556ccd9cab7f2f8ce4597109898b

(and I wouldn't say this is a regression, but just a bug)

[johannschopplich]

Tough topic. But somehow I'm glad to hear this issue came up again. Honestly, I'm always using the Symfony parser along with Kirby. I'm not an expert regarding YAML syntax and its variants; just wanna share my experience, while I understand the problems such a breaking change for every blueprint out there would bring.

Nowadays, with a VSCode extension for YAML, every loose syntax currently allowed by Spyc (like unquoted asterisks) will throw an error in VSCode. Thus, the correct YAML syntax is enforced intrinsically, at least for me. And the Symfony parser handled every blueprint splendid. I had no problems in any Kirby project.

This is what I throw into Kirby's composer.json:

{
  "autoload": {
    "psr-4": {
      "Kirby\\": "site/app/"
    },
    "exclude-from-classmap": [
      "vendor/getkirby/cms/src/Data/Yaml.php"
    ]
  },
}

... Given the parser inside site/app/Data/Yaml.php.

[neildaniels]

Unless someone intends on forking Spyc, “modernizing” and then maintaining it, I think continuing to rely on it is probably a dead-end.

I know YAML is used in a number of places in Kirby (Blueprints, Structure values, Blocks, etc.), so I think its hard to grasp how big of a change this would be. An inventory of Yaml usage and what would be “breaking” would be helpful.

I think reading values from content files (structure values and Blocks) should really be treated as something that simply need to keep working. Blueprints, IMO, are much more reasonable to have to migrate, if necessary.

What if we tip-toe into the Symfony direction by having both libraries. When in debug mode (or some other option flag), values are decoded with both libraries and differences throw a deprecation warning, but the Spyc version is still actually used in all cases. During this initial phase, I would also continue to just use the Spyc encoder.

Then, later, start using the Symfony decoder and encoder. The Spyc library maybe can continue to stick around as a fallback decoder, if it’s necessary to read old content file values.

(As of now, it seems like using the Spyc exclusively for parsing Yaml is not [as?] prone to throwing deprecation warnings.)

[bastianallgeier]

✅

[bastianallgeier]

We decided to buy us time by starting to modernize at least some parts of Spyc. The move to a new handler is definitely necessary, but it's also more complex than we wished.