Problem/Motivation

There's a @todo in \Drupal\Core\TypedData\TypedData::__construct() which says that we should use a type hint on DataDefinitionInterface for the $definition argument when #1928868: Typed config incorrectly implements Typed Data interfaces is fixed.

Proposed resolution

Since the referenced issue has been fixed for a while, let's also fix the @todo :)

Remaining tasks

Write a patch.

User interface changes

Nope.

API changes

Nope.

Data model changes

Nope.

CommentFileSizeAuthor
#5 2978788-type-hinting-1.patch989 byteseli-on-drupal
#3 2978788-type-hinting.patch936 byteseli-on-drupal

Comments

amateescu created an issue. See original summary.

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +DX (Developer Experience)

Nice!

eli-on-drupal’s picture

eli-on-drupal commented
Status: Active » Needs review
StatusFileSize
new 2978788-type-hinting.patch936 bytes

I added the type hint and removed the @todo for the issue.

amateescu’s picture

amateescu commented

Thanks, @eliclaggett, looks great! Just a tiny coding style issue left:

+++ b/core/lib/Drupal/Core/TypedData/TypedData.php
@@ -60,11 +60,8 @@ public static function createInstance($definition, $name = NULL, TraversableType
    * @see \Drupal\Core\TypedData\TypedDataManager::create()
    *

There's an extra empty line here that should be removed, just after the line with @see.

eli-on-drupal’s picture

eli-on-drupal commented
StatusFileSize
new 2978788-type-hinting-1.patch989 bytes

Oops, here is a fixed version.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

Perfect!

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

This is just a constructor change so fine to do.

Committed 5983353 and pushed to 8.6.x. Thanks!

  • catch committed 5983353 on 8.6.x 
    Issue #2978788 by eliclaggett, amateescu: \Drupal\Core\TypedData\...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.