Convert FieldType plugin discovery to attributes

Created on 13 February 2024, 10 months ago
Updated 9 May 2024, 8 months ago

Problem/Motivation

In πŸ“Œ Use PHP attributes instead of doctrine annotations Fixed we added support for attribute based plugin discovery.
As part of that issue we converted block and action plugins.

This issue is to convert \Drupal\Core\Field\Annotation\FieldType plugins to use Attributes.

Proposed resolution

  1. Add a class to represent the new Attribute - Example
  2. Update the plugin manager constructor to include both the attribute and annotation class names - example
  3. Convert all plugins that use the annotation to use the new attribute - example

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ“Œ Task
Status

Fixed

Version

11.0 πŸ”₯

Component
EntityΒ  β†’

Last updated about 11 hours ago

Created by

πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @larowlan
  • Status changed to Postponed 10 months ago
  • Status changed to Active 10 months ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    πŸ“Œ Convert DataType plugin discovery to attributes Fixed is in so this is unblocked.

  • Status changed to Needs review 10 months ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia
  • Pipeline finished with Success
    10 months ago
    Total: 527s
    #109867
  • Status changed to Needs work 10 months ago
  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    Can we update the docs for hook_field_info_alter in field.api.php here too? There are references to the annotation class but we should update to the attribute class.

  • Status changed to Needs review 10 months ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    #6 Done.

    I was also questioning how these worked with Annotation\FieldType as they weren't defined anywhere I can see:

        public readonly array $config_dependencies = [],
        public readonly array $column_groups = [],
        public readonly array $serialized_property_names = [],
    
  • Pipeline finished with Success
    10 months ago
    Total: 629s
    #109892
  • Status changed to Needs work 10 months ago
  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    #7 I think we can remove those. They are the additional keys mentioned at the top of the attribute/annotation class. Since the attribute values get converted to an array they don't need to be defined.

  • πŸ‡¦πŸ‡ΊAustralia mstrelan

    Actually I think if we need to remove them then we need an alter hook to add the values in.

  • πŸ‡¨πŸ‡­Switzerland berdir Switzerland

    What I'm doing in the entity type issue is add an explicit additional key where you can put it arbitrary settings.

    But some of these we probably should define if they're part of the official API.

    serialized property names is part of a security issue that went in a while ago, column_groups is afaik mostly used for content _translation, not sure about config dependencies. It's used in \Drupal\Tests\field\Kernel\TestItemWithDependenciesTest, would probably need to check the issue that added this to see what the real use cases are.

  • Pipeline finished with Success
    8 months ago
    Total: 1079s
    #148708
  • Status changed to Needs review 8 months ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    @Berdir #10 i had a look at πŸ“Œ Convert entity type discovery to PHP attributes Needs review and doesn't look this you are doing this anymore?

    I have left the properties in for now.

    Re: config_dependencies I found this in the docs for \Drupal\Core\Config\Entity\ConfigDependencyManager

     * Classes for configurable plugins are a special case. They can either declare
     * their configuration dependencies using the calculateDependencies() method
     * described in the paragraph above, or if they have only static dependencies,
     * these can be declared using the 'config_dependencies' annotation key.
  • Status changed to RTBC 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Applied the MR searched for @FieldType all instances have been replaced
    Verified $deriver is there (something had to check from previous attribute tickets)
    Tests are all green, showing nothing broke.

    Believe this is good

  • Status changed to Needs work 8 months ago
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ
  • Pipeline finished with Canceled
    8 months ago
    Total: 844s
    #153792
  • Pipeline finished with Failed
    8 months ago
    Total: 185s
    #153804
  • Pipeline finished with Success
    8 months ago
    Total: 1018s
    #153810
  • Status changed to Needs review 8 months ago
  • πŸ‡¦πŸ‡ΊAustralia kim.pepper πŸ„β€β™‚οΈπŸ‡¦πŸ‡ΊSydney, Australia

    Addressed feedback so back to NR

  • Status changed to Needs work 8 months ago
  • The Needs Review Queue Bot β†’ tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide β†’ to find step-by-step guides for working with issues.

  • First commit to issue fork.
  • Pipeline finished with Success
    8 months ago
    Total: 502s
    #156410
  • Status changed to Needs review 8 months ago
  • πŸ‡³πŸ‡ΏNew Zealand quietone
  • Status changed to RTBC 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    All feedback appears to be addressed here.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Committed and pushed cf09596544 to 11.x and 4afc682d90 to 10.3.x. Thanks!

    • alexpott β†’ committed 4afc682d on 10.3.x
      Issue #3420981 by kim.pepper, quietone, mstrelan, Berdir, alexpott:...
  • Status changed to Fixed 8 months ago
    • alexpott β†’ committed cf095965 on 11.x
      Issue #3420981 by kim.pepper, quietone, mstrelan, Berdir, alexpott:...
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024