SPIKE Review third party dependencies

[maxime-rainville]

There's a thirdparty dependencies in framework that contains a bunch of weird things that probably shouldn't exists.

Timebox

• 1 day

Objectives

• Review all the dependencies managed via the thirdparty folder
• For each one, identified an approach to managing them going forward:
  • Fold it back into our codebase
  • Spine them out into seperate module
  • Swap for a better more modern equivalent
  • Something else
• Where we expect to take full ownership of the thirdparty dependency, we should scope the work needed to provide reasonable test coverage
• Create matching cards for each dependency

[GuySartorelli]

Initial thoughts here.

difflib (see #10640)

• The original code doesn't seem to be on github
• Maybe replace with https://github.com/sebastianbergmann/diff
• Alternatively several other options have been mentioned in ATTENTION! Don't use this package and any forks, use sebastian/diff instead chrisboulton/php-diff#53
• Last option would be to fold it into our codebase but we really shouldn't have to maintain and support our own library for this.

Used by:

• We made our own SilverStripe\View\Parsers\Diff subclass of it
• SiteTreeTest references it
• DataDifferencer uses it to get differences between copies of HTML content
• VersionedAdmin's DiffField (who knew we had that?) uses it to get differences between copies of HTML content

php-peg (see #10644)

• The original is at https://github.com/hafriedlander/php-peg but is super abandoned (no updated since 2014)
• Of the available forks, this one seems to be maintained but it is self proclaimed as "somehow opinionated" which worries me a little.
• I'll spend a small amount of time looking into that fork, but ultimately it may be better for peace of mind to just fold this into our codebase and accept that we have to live with it until such a time as we can ditch the concept of *.ss templates.

Simpletest (see #10642)

• This is only ever directly used by TestSession::lastPage() which is used by TestSession::SubmitForm().
• I think it should be relatively easy to just replace this with something modern (there's probably already something in our codebase or in phpunit)

[maxime-rainville]

difflib

My preference would be swap it with something modern. If it's not possible to do it in time for the beta, my instinct would be to declare it an internal API and not support it officially. Alternatively, we could create an interface in front of SilverStripe\View\Parsers\Diff and support that instead of the underlying implementation.

php-peg

I'm OK with folding it in. Again, I think it makes sense to declare class @internal if we don't want people to interact with them directly.

Simpletest

Presuming we don't expose any of those classes through our own API, I think it makes total sense to declare this an unsupported dep. Which means we could remove it post beta if wanted/needed to.

[maxime-rainville]

Presuming there's no added complexity with difflib, I'm happy to call this SPIKE done.

[GuySartorelli]

Difflib

My POC PR for difflib shows that it should be possible to swap out with a modern alternative fairly easily - the main problem we're gonna have I think is not mangling the html in the process (difflib is a bit odd in that it marks replacements, where most diff libraries I've seen just mark them as separate added/removed which makes it harder to keep the HTML in once piece) - but I think with some careful thought we can deal with that in a day or two at most.

But yeah if that takes too long it's also easy enough to just move that logic into our SilverStripe\View\Parsers\Diff class, which is how I'd recommend folding it in if we were to fold it in.

php-peg

I've added an AC to the php-peg issue to mark the classes as @internal.

simpletest

simpletest is only used by SessionTest but we do return a simpletest "page" object from a public method on that class. So it's technically part of our public API at the moment.