dustin10 / VichUploaderBundle

A simple Symfony bundle to ease file uploads with ORM entities and ODM documents.
MIT License
1.85k stars 519 forks source link

Throw MissingPackageException if doctrine/annotations is not installed #1355

Closed andyexeter closed 1 year ago

andyexeter commented 1 year ago

As per the discussion in #1344 - this PR throws a MissingPackageException during container compilation if the doctrine/annotations package is not installed and the metadata type config is set to annotation or attribute.

Given that the default value for this config is attribute, developers not using attributes or annotations and relying on the default would need to update their config to set the value to null e.g.:

vich_uploader:
    metadata:
        type: ~
garak commented 1 year ago

Given that the default value for this config is attribute, developers not using attributes or annotations and relying on the default would need to update their config to set the value to null e.g.:

This is a breaking change, so we can't merge it in the current master (which is meant for the next patch/minor) version. Moreover, the plan is to make attribute support independent from the annotations library, so I'm afraid this proposal is currently useless.

andyexeter commented 1 year ago

I understand that, however the bundle is already broken in its current state because its default configuration (attributes) relies on a package which it does not require in its composer.json

garak commented 1 year ago

I understand that, however the bundle is already broken in its current state because its default configuration (attributes) relies on a package which it does not require in its composer.json

Yes, but it's only broken if you upgrade Doctrine to a version that doesn't require annotations. If we merge this, it will break for existing users only by upgrading this bundle, which is worse.

andyexeter commented 1 year ago

Not just upgrades. Anyone creating a new project and requiring this bundle will not be able to use its default configuration without also requiring the doctrine/annotations package.

Given that the default configuration for this bundle is to use attributes, perhaps the solution for now is to require the doctrine/annotations package in composer.json

garak commented 1 year ago

Not just upgrades. Anyone creating a new project and requiring this bundle will not be able to use its default configuration without also requiring the doctrine/annotations package.

That's true, but a new project is a project that was not working before. So, a new user should read carefully the documentation (where we already put a warning on the question). Instead, breaking an old project means making unusable something that was previously working.

Given that the default configuration for this bundle is to use attributes, perhaps the solution for now is to require the doctrine/annotations package in composer.json

That would solve, but it's too much: annotations are not needed if you don't use annotations or attributes.