fix: enable compatibility with symfony 7.3 + PHP 8.4 #891
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
512581c to
74a577b
Compare
nikophil
commented
May 4, 2025
Comment on lines
+117
to
118
| 'use \Symfony\Component\VarExporter\Internal\LazyDecoratorTrait' => \sprintf("use \\%s;\n use \\%s", IsProxy::class, LazyProxyTrait::class), | ||
| 'use \Symfony\Component\VarExporter\LazyProxyTrait' => \sprintf("use \\%s;\n use \\%s", IsProxy::class, LazyProxyTrait::class), |
There was a problem hiding this comment.
symfony/var-exporter used to generate code like this:
class ZenstruckFoundryTestsFixtureEntityAddressProxy extends \Zenstruck\Foundry\Tests\Fixture\Entity\Address implements \Zenstruck\Foundry\Persistence\Proxy, \Symfony\Component\VarExporter\LazyObjectInterface
{
use \Symfony\Component\VarExporter\LazyProxyTrait;and then, we were replacing use \Symfony\Component\VarExporter\LazyProxyTrait by
use \Symfony\Component\VarExporter\LazyProxyTrait;
use IsProxy;but now, with php 8.4 + sf 7.3 (and only for this "combo"), it generates:
class ZenstruckFoundryTestsFixtureEntityAddressProxy extends \Zenstruck\Foundry\Tests\Fixture\Entity\Address implements \Zenstruck\Foundry\Persistence\Proxy, \Symfony\Component\VarExporter\LazyObjectInterface
{
use \Symfony\Component\VarExporter\Internal\LazyDecoratorTrait;so let's replace both occurrences
nikophil
commented
May 4, 2025
| public function it_can_generate_proxy_for_class_with_self_return_type(): void | ||
| { | ||
| $proxyfiedObj = ProxyGenerator::wrap($obj = new ClassWithSelfReturnType()); | ||
| self::assertSame($obj, $proxyfiedObj->returnsSelf()->_real()); // @phpstan-ignore method.notFound |
There was a problem hiding this comment.
this is a change in sf 7.3: now methods which returns self/$this/static can now return a proxy
I think this is kinda nice, but I'm expecting some issues with subtle bugs poping because of this 🤷
nikophil
commented
May 4, 2025
Comment on lines
+123
to
+141
| /** | ||
| * Add `$this->_autoRefresh();` after every method declaration. | ||
| * | ||
| * (\s* # 1. Optional indentation | ||
| * (?:public|protected|private)?\s* # 2. Optional visibility | ||
| * function\s+ # 3. The "function" keyword followed by space | ||
| * (?!__) # 4. Negative lookahead to exclude magic methods (like __serialize) | ||
| * \w+ # 5. Method name | ||
| * \s*\([^\)]*\)\s* # 6. Parameters inside parentheses (not captured) | ||
| * ):?\s*\??[\w\\\\]* # 7. Optional return type, can be nullable (starts with `?`), supports namespaced types (`\Foo\Bar`) | ||
| * \s*\{\s*$ # 8. Opening brace `{` at the end of the line, with optional spaces before | ||
| */ | ||
| $proxyCode = preg_replace_callback( | ||
| '/^(\s*(?:public|protected|private)?\s*function\s+(?!__)\w+\s*\([^\)]*\)\s*):?\s*\??[\w\\\\]*\s*\{\s*$/m', | ||
| function ($matches) { | ||
| return rtrim($matches[0]) . "\n \$this->_autoRefresh();"; | ||
| }, | ||
| $proxyCode | ||
| ); |
There was a problem hiding this comment.
I had to rework how we introduce the call to _autoRefresh()
symfony/var-exporter used to generate code like this for any method:
public function getContact(): ?\Zenstruck\Foundry\Tests\Fixture\Entity\Contact
{
if (isset($this->lazyObjectReal)) {
return ($this->lazyObjectState->realInstance ??= ($this->lazyObjectState->initializer)())->getContact(...$this->unproxyArgs(\func_get_args()));
}
return parent::getContact(...$this->unproxyArgs(\func_get_args()));
}so we were relying on the if() to introduce the call. But now, with php 8.4 + sf 7.3 (again, only for this combo), there is no more if, so I changed how it worked to use a regex which looks for any method declaration
Sorry for the complex regex, but I asked the AI to help me here
kbond
approved these changes
May 5, 2025
Kocal
added a commit
to symfony/ux
that referenced
this pull request
Aug 6, 2025
…yImage][LiveComponent][Map][Notify][React][StimulusBundle][Svelte][Swup][TogglePassword][Toolkit][Translator][Turbo][TwigComponent][Typed][Vue] Add support for Symfony 8 (Kocal) This PR was merged into the 2.x branch. Discussion ---------- [Autocomplete][Chartjs][Cropperjs][Dropzone][Icons][LazyImage][LiveComponent][Map][Notify][React][StimulusBundle][Svelte][Swup][TogglePassword][Toolkit][Translator][Turbo][TwigComponent][Typed][Vue] Add support for Symfony 8 | Q | A | ------------- | --- | Bug fix? | no | New feature? | yes <!-- please update src/**/CHANGELOG.md files --> | Docs? | no <!-- required for new features --> | Issues | Fix #... <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead --> | License | MIT <!-- Replace this notice by a description of your feature/bugfix. This will help reviewers and should be a good start for the documentation. Additionally (see https://symfony.com/releases): - Always add tests and ensure they pass. - For new features, provide some code snippets to help understand usage. - Features and deprecations must be submitted against branch main. - Update/add documentation as required (we can help!) - Changelog entry should follow https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry - Never break backward compatibility (see https://symfony.com/bc). --> As suggested in symfony/webpack-encore-bundle#248, I added new jobs for testing explicit Symfony version. The Symfony 8.0.x-dev job is failing because it needs zenstruck/foundry#891, which is not possible to use because Foundry is not compatible with Symfony 8 yet (opened zenstruck/foundry#960) There is a lot of changes, but these are only small modifications (composer.json, CHANGELOG.md, and fixing method typings) I also updated the deprecated packages, that's fine, it makes things easier for us to handle Commits ------- 998de8d Add support for Symfony 8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #890
Here is a fix when using Symfony 7.3 AND PHP 8.4
since symfony/symfony#59890 there was some changes in var-exporter, to support new php proxy system.
and because we're doing weird things, which deals with extra internal stuff with proxy system, things was failing 🤷
I think this is a very short term fix, since it won't be possible to rely on Symfony's
ProxyHelperfor Foundry's proxyh system. We'd need to find another way to do this