Allow symfony 7 dependencies

[weaverryan]

Hi!

Allowing Symfony 7 dependencies to be installed.

This requires a BC break: to stop using the deprecated ContainerAwareLoader.

Do we want to TRY to have a BC-layer? It would require, I think, some config where the user "turns off" the ContainerAwareLoader to silence the deprecation. Honestly, I don't think anyone has used container-aware loaders for a long time - requiring an option sounds annoying for users. I'd vote for a new 4.0 with this change, but I'll defer to what others think is best.

If we did a new major, we could also very easily drop old php & Symfony versions.

Cheers!

[greg0ire]

Is this a breaking change? How will it affect people still using Symfony 6? According to the deprecation message embedded in the bridge class, people should first "use dependency injection in [their] fixtures instead".

[greg0ire]

Ah, just saw you edited the description :)

[greg0ire]

Honestly, I don't think anyone has used container-aware loaders for a long time - requiring an option sounds annoying for users.

I'm not sure this is the point: people relying on container-aware fixtures get the very same deprecation as people not relying on it. It might have been better to wrap the injected container with a wrapper that triggers a deprecation whenever somebody calls get() or has() on that container, but I guess that ship has sailed :)

I think there is a way to do this without a breaking change though:

1. create a file that conditionally declares a class, depending on whether ContainerAwareLoader exists or not.

if (class_exists(ContainerAwareLoader)) {
    class IntermediaryContainer extends ContainerAwareLoader {}
} else {
    class IntermediaryContainer extends Container {}
}

2. extend that class in SymfonyFixturesLoader

As a result, people with Symfony 6 will not be affected, and people with Symfony 7 cannot complain since they got a deprecation about this issue.

The deprecation is used in an execution path that is unlikely to be overused, and I agree that developing something to turn it off might be overkill.

[weaverryan]

Thanks for considering this with me :). If we do the conditional class:

if (class_exists(ContainerAwareLoader)) {
    class IntermediaryLoader extends ContainerAwareLoader {}
} else {
    class IntermediaryLoader extends Loader {}
}

Won't people using Symfony 6.4 get a deprecation warning that they can't silence? The ContainerAwareLoader class will exist, so it'll be extended, and that'll trigger the deprecation warning on that class, right? If I am, we would need some bundle config to "disable" extending that class. That's the part that sounds annoying for the 99% of users that probably haven't used container-aware loaders for awhile.

[greg0ire]

Fully agree with you, but:

The deprecation is used in an execution path that is unlikely to be overused, and I agree that developing something to turn it off might be overkill.

That being said, I just pushed 4.0.x, because in any case, there will need to be a contribution on that branch soon (either a cleanup, or we just retarget your PR).

Ideally, it would be cool to have a way for users to opt-out, but I can't see how to embed such a condition inside if (class_exists(ContainerAwareLoader)) 🤔 . I think such situations will always happen if we use extends and a class from another package.

[weaverryan]

I've just retargeted the branch to 4.0.x. It is, at least, a good starting point. For a new major, we can also drop old php & Symfony versions. Keep that separate from this PR or embed it?

[greg0ire]

Let's go with separate PRs :)

[OrestisZag]

@greg0ire @weaverryan Great work with this. 😄 Are you gonna tag this soon or is there some other issue? Thank you!

[derrabus]

We don't plan to release 4.0 anytime soon. Use 3.5.

[OrestisZag]

@derrabus thanks for your answer. This is needed for Symfony 6.4, right? Or is there another way to proceed?

[derrabus]

Or is there another way to proceed?

There is. Use 3.5.