Skip to content

[JsonEncoder] Add JsonEncodable attribute #59401

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Jan 7, 2025

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

Add JsonEncodable attribute that autoconfigures json_encoder.encodable tag.

@OskarStark
Copy link
Contributor

AsEncodableItem? Otherwise it does not match our As... naming or just "Encodable"?

@mtarld mtarld force-pushed the feat/encodable-attribute branch 3 times, most recently from 0e5c24d to b70b7ea Compare January 7, 2025 15:53
@mtarld mtarld changed the title [JsonEncoder] Add AsEncodable attribute [JsonEncoder] Add Encodable attribute Jan 7, 2025
@mtarld mtarld force-pushed the feat/encodable-attribute branch 4 times, most recently from 0c4c28b to 073610e Compare January 7, 2025 16:24
@mtarld mtarld force-pushed the feat/encodable-attribute branch 2 times, most recently from ad8e5a0 to 962ae14 Compare January 8, 2025 14:23
@mtarld mtarld force-pushed the feat/encodable-attribute branch from 962ae14 to 60fd915 Compare January 8, 2025 14:27
@mtarld mtarld changed the title [JsonEncoder] Add Encodable attribute [JsonEncoder] Add JsonEncodable attribute Jan 8, 2025
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I like how we're using the container loading tools to do resource discovery quite seamlessly.

@mtarld mtarld force-pushed the feat/encodable-attribute branch from 60fd915 to a12ad34 Compare January 13, 2025 09:21
@chalasr
Copy link
Member

chalasr commented Jan 15, 2025

Thank you @mtarld.

@chalasr chalasr merged commit d096737 into symfony:7.3 Jan 15, 2025
11 checks passed
@mtarld mtarld deleted the feat/encodable-attribute branch January 15, 2025 20:58
nicolas-grekas added a commit that referenced this pull request Feb 11, 2025
…)` and `ContainerBuilder::findExcludedServiceIds()` for auto-discovering value-objects (GromNaN)

This PR was squashed before being merged into the 7.3 branch.

Discussion
----------

[DependencyInjection] Add `Definition::addExcludedTag()` and `ContainerBuilder::findExcludedServiceIds()` for auto-discovering value-objects

| Q             | A
| ------------- | ---
| Branch?       | 7.3
| Bug fix?      | no
| New feature?  | yes
| Deprecations? | no
| Issues        | -
| License       | MIT

We could **not** use the method `findTaggedServiceIds` in #59401 (comment), same for api-platform/core#6943.

As "using the container loading tools to do resource discovery quite seamlessly" [seems to be a good idea](#59401 (review)), this changes make it easier.

I'm not closed to alternative ideas if we want to go further with this use-case.

### Usage

Let's create a `AppModel` attribute class and use it on any class of the project.

In the extension class:
```php
$this->registerAttributeForAutoconfiguration(AppModel::class, static function (ChildDefinition $definition) {
    $definition->addExcludedTag('app.model');
});
```

In a compiler pass:
```php
$classes = [];
foreach($containerBuilder->findExcludedServiceIds('app.model') as $id => $tags) {
    $classes[] = $containerBuilder->getDefinition($id)->getClass();
}

$containerBuilder->setParameter('.app.model_classes', $classes);
```

And this parameter can be injected into a service, or directly update a service definition to inject this list of classes. The attribute parameters can be injected into the tag, and retrieved in the compiler pass, for more advanced configuration.

Commits
-------

7a0443b [DependencyInjection] Add `Definition::addExcludedTag()` and `ContainerBuilder::findExcludedServiceIds()` for auto-discovering value-objects
@fabpot fabpot mentioned this pull request May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants