Slowly, very slowly start OOPifying the render system

dww → credited andypost → .

dww → credited kingdutch → .

dww → credited moshe weitzman → .

It looks great idea to git in for D12!

btw it will raise a question of DI for elements as related still)

I converted FieldThirdPartyTestHooks as well so we convert something that isn't a form_alter.

I think a couple of examples outside of tests would be good too.

The functional JS test was failing, in hindsight it was kind of obvious what I missed.

I passed the objects back into the array assuming that toRenderArray would be called as necessary upstream. The idea that @moshe had to add a key property we can set and return the object directly would be nice too.

I got InvalidArgumentException: "field_test_field_formatter_third_party_settings_form" is an invalid render array key. Value should be an array but got a object. in Drupal\Core\Render\Element::children() (line 97 of /var/www/html/core/lib/Drupal/Core/Render/Element.php).
when running through the test manually.

For this particular test: https://git.drupalcode.org/project/drupal/-/merge_requests/12173/diffs?c... fixes it by converting back to the render array.

dww → credited godotislate → .

By request from chx in Slack, adding more credits

One test failure:
Drupal\Tests\system\Functional\Form\ArbitraryRebuildTest::testUserRegistrationRebuild
Behat\Mink\Exception\ElementNotFoundException: Button with id|name|label|value "Rebuild" not found.

Don't think I've seen this one as an intermittent failure.

First real look at the MR. Mostly nits. A few questions of substance. Out of time for now. More later...

phpcs won, I am done.

@chx: really? Please. The drama is helping no one. phpcs is a tool. It’s not hard to use. You can configure your IDE to get it right. You don’t need to take it so personally and get so worked up and angry over it. Please take some deep breaths, and maybe a day off, but spare us the public agony and tantrum over how having standards is an impediment to velocity.

We’ve been in this together for nearly 2 decades now. You know how much respect I have for you and your abilities. Please know I’m only writing this out of love and care. I invite you to relax and have some perspective.

Thanks,
-Derek

I've added skip rules for a bunch of jobs while we continue to work on architecture, maybe it's worth configuring this so draft MRs don't run these tests by default so that the actual architecture can happen unimpeded, then once the MR is no longer draft we can fix phpcs etc.

I've also added the fix for the fromClass work. I tried running the test locally but for some reason they wouldn't start.

Turns out core seems to ignore these flags, I just set them to allow failure.

Added a test that swaps out a class and added something to attempt to make the standalone wrappers nicer.

There are several failures, we could possibly revert that, but let's think on it a bit.

I updated the IS as well with some next steps, this could use a pass on comments, and we need to finish tests and tidy up the deprecations.

Adding just in case

Thanks @nod_

I removed the skip since it doesn't work anyway here is the issue to track: 📌 Consider allowing first stage tests to fail when an MR is in draft. Active

Ok this needs some additional tests, but we are in a good spot for another review.

I think this is ready for review again, we've added tests and addressed most of the feedback.

I also added the deprecation.

Beyond review it might be worth identifying a few more things to convert.

Thank you for the reviews! The latest round of comments should all be addressed. I left a few open that still need confirmation that the answers are acceptable.