Problem/Motivation
Running the tests against Core 9.1 with deprecations un-supressed produces the deprecation warning
Declaring ::setUp without a void return typehint in {...} is deprecated in drupal:9.0.0. Typehinting will be required before drupal:10.0.0. See https://www.drupal.org/node/3114724
The Change Record is https://www.drupal.org/node/3114724
There are 62 warnings, all with the same message, produced in the following tests:
typed_data\Kernel\PlaceholderResolverTest
typed_data\Kernel\DataFetcherTest
typed_data\Kernel\DataDefinitionFetcherTest
typed_data\Kernel\DataFilterTest
typed_data\Kernel\DataFetcherAutocompleteTest
typed_data\Functional\TypedDataFormWidget\BrokenWidgetTest
typed_data\Functional\TypedDataFormWidget\SelectWidgetTest
typed_data\Functional\TypedDataFormWidget\TextInputWidgetTest
typed_data\Functional\TypedDataFormWidget\TextareaWidgetTest
The message says that this is deprecated in 9.0.0 but this warning is not produced at 9.0, neither on drupal.org nor travis testing. It is only shown for core 9.1
Proposed resolution
Remaining tasks
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3165602-11.add-void-typehint.patch | 6.48 KB | jonathan1055 |
Comments
Comment #2
jonathan1055 commentedThis patch changes all the Kernel and Functional test
protected function setUp() {toprotected function setUp(): void {. Howeverm this change also requires PHP7.1 as it fails on 7.0. According to https://www.drupal.org/docs/system-requirements/php-requirements PHP7.0 reach end-of-life at end of 2018, and php7.2 is recommended for D8. This only affects our testing environments, so we can take a view on whether we still want to test at PHP7.0Here's the patch anyway, which includes a drupalci.yml with
suppress-deprecations: falseto show that this fixes the problem.Comment #4
jonathan1055 commentedand run the tests :-)
Comment #5
jonathan1055 commentedThat all worked well. Here is the actual patch to commit, i.e without the addition of drupalci.yml
Comment #6
tree2009 commentedComment #7
tr commentedThis is something we can only really change in D9. In order for this to work in D8 it requires that Typed Data API increase the PHP requirement to 7.1, which is higher than Drupal core PHP requirement. I'm strongly against imposing requirements different than Drupal core.
There's a BC layer for this in D9.0, so the deprecation only starts to affect us in D9.1, and this hint will be required only in D10.
This is also an API change, so it should not done in a minor point release of the Typed Data API. What I would like to do it get an 8.x-1.0 version stable release first, which will continue to be the branch that supports D8 and D9.0. After that I would like to open up a 2.0.x semantic versioning branch where we can support API changes like this which are being introduced in D9.1+.
Comment #8
jonathan1055 commentedOK I get your point about not wanting impose a higher php than the absolute minimum that drupal supports, even if that version reached end-of-life at end of 2018. But this only affects our testing environments, doesn't it? We do not need to require php7.1 for Typed Data in regular use. It's just that we (as developers) are much more likely to be running at least php7.1. The current tests configued for Typed Data on drupal.org do not use php7.0 the lowest is 7.1. I do not have php7.0 locally so I can't verify this, but so long as you do not try to run the automated tests I would guess that Typed Data can be installed on php7.0 with the test file changes? Or maybe I'm wrong.
Comment #9
tr commentedOK, I guess I can live with this since it's just in the tests and we don't test against 7.0 and don't plan to test against 7.0 ever.
Could you update the changes to .travis.yml and re-roll this, so we don't have to make more follow-up changes to .travis.yml after I commit this issue?
Comment #10
jonathan1055 commentedHere's a re-roll of patch #5. Also needed to add the fix for the new test
DatetimeWidgetTest. Travis update also included.Following the commit on #3169351: [9.1] $modules property must be declared protected and this with one, we'll have zero deprecations at 9.1. I will update #3042684: [meta] Deprecated code in Typed Data when you have done this commit.
Comment #11
jonathan1055 commentedMinor change to remove duplication of echo statement
Comment #13
tr commentedThanks. Committed.
Comment #14
jonathan1055 commentedNice.
Comment #15
jonathan1055 commentedArgh, stale form data? The url was opened from before you fixed, and I refreshed the page. It showed "fixed" from your comment, but when I made my comment the status changed back to 'needs review'.