Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21TypedData...
$configuration['name'] and $configuration['parent'] are not optional because they are used unconditionally in TypedDataManager::createInstance(). So omitting these options causes 'Undefined index' notices.
$typed_data = $class::createInstance($data_definition, $configuration['name'], $configuration['parent']);
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2592213-8.patch | 1.4 KB | hussainweb |
| #8 | interdiff-4-8.txt | 1.38 KB | hussainweb |
Comments
Comment #2
Chi CreditAttribution: Chi commentedComment #3
Chi CreditAttribution: Chi commentedComment #4
Chi CreditAttribution: Chi commentedFixed wrapping.
Comment #5
chx CreditAttribution: chx at Smartsheet commentedOh nice!
Comment #6
jordanpagewhite CreditAttribution: jordanpagewhite as a volunteer commentedIt looks like the only call to TypedDataManager::createInstance() is TypedDataManager::create, which sets passes NULL to configuration['name'] and configuration['parent'] by default. So, it seems like passing 'name' and 'parent' is still technically optional, but I do think that this is confusing. At the very least, maybe we can add 'Set to NULL by default' at the end of each of those two lines. We could even add a @see Drupal\Core\TypedData\TypedDataManager::create . What do you think?
Comment #7
chx CreditAttribution: chx at Smartsheet commentedSo yeah, this is confusing , we should add "can be NULL but must be set" or something to that effect.
Comment #8
hussainwebHow about this? I changed the sentence order to make the text flow.
Comment #9
chx CreditAttribution: chx at Smartsheet commentedI like it.
Comment #10
jordanpagewhite CreditAttribution: jordanpagewhite as a volunteer commentedI'm still not sure that this is the most accurate description of the current state of the code. What would happen if someone didn't pass ['name'] or ['parent']? Unless I am misunderstanding, it seems like (since createInstance is only called by create) that everything would be fine since $name = NULL and $parent = NULL in the create method declaration. If I am misunderstanding, I would really like to know though. I am trying to better understand core so I can be a valuable contributor.
I'm also wondering if it was an architectural decision to only call createInstance in the create method.
Comment #11
alexpott@jordanpagewhite if createInstance is called from somewhere else then 'name' and 'parent' must exist - this patch is just documenting that.
We could argue that we're redefining a public API here but I think it is better that documentation is correct rather then worrying about the semantics of semver in this instance. Committed 88233eb and pushed to 8.0.x and 8.1.x. Thanks!