Skip to content

[LiveComponent] Fix PropertyTypeExtractorInterface::getTypes() deprecation, use TypeInfo ^7.2 Type #2607

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
May 30, 2025

Conversation

mtarld
Copy link
Contributor

@mtarld mtarld commented Feb 26, 2025

Q A
Bug fix? no
New feature? yes
Issues Fix #1708, fix #2778
License MIT

Fix the PropertyTypeExtractorInterface::getTypes() deprecation and use PropertyTypeExtractorInterface::getType() (based on TypeInfo Type) instead.

This PR allows TypeInfo ^7.2 to be used, thanks to the compatibility layers added for Type::accepts() and Type::traverse() (added in 7.3). In order to reduce frictions when users will upgrade their apps dependencies.

The CI changed a bit too:

  • When testing LiveComponent, with PHP 8.2, we install symfony/property-info:7.1.* symfony/type-info:7.2.*
  • When testing LiveComponent, with PHP 8.3, we install symfony/property-info:7.2.* symfony/type-info:7.2.*
  • When testing LiveComponent, with PHP 8.4, we install symfony/property-info:7.3.* symfony/type-info:7.3.*
  • When testing LiveComponent, with PHP 8.4 (and dev deps), we install symfony/property-info:>=7.3 symfony/type-info:>=7.3

Allowing us to covers a maximum versions of PropertyInfo and TypeInfo.

@mtarld mtarld force-pushed the feat/type-info-type branch 3 times, most recently from 87a72aa to d3edb38 Compare February 26, 2025 17:16
@mtarld
Copy link
Contributor Author

mtarld commented Feb 26, 2025

Actually, didn't notice that PHP 8.1 is still supported by Symfony UX.
Unfortunately, it'll prevent symfony/type-info to be installed in that version.

@mtarld mtarld force-pushed the feat/type-info-type branch 3 times, most recently from 8777e0e to 81650fd Compare February 26, 2025 17:36
@Kocal Kocal added this to the 3.x milestone Feb 26, 2025
@mtarld
Copy link
Contributor Author

mtarld commented Feb 26, 2025

Anyway, I think we must wait for the bump of PHP in composer.json (ie: symfony/ux-live-component:^3.0).

@Kocal
Copy link
Member

Kocal commented Feb 26, 2025

Added milestone 3.x

chalasr added a commit to symfony/symfony that referenced this pull request Mar 22, 2025
This PR was merged into the 7.3 branch.

Discussion
----------

[PropertyInfo] Deprecate `Type`

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

A new attempt to #53160, now that `symfony/type-info` is not experimental anymore.

Deprecates:
- `Type` class in favor of the `Type` class of `symfony/type-info`
- `PropertyTypeExtractorInterface::getTypes()` in favor of the `PropertyTypeExtractorInterface::getType()` method
- `ConstructorArgumentTypeExtractorInterface::getTypesFromConstructor()` in favor of the `ConstructorArgumentTypeExtractorInterface::getTypeFromConstructor()` method

The work for upgrading dependent packages has begun already:
- api-platform/core#6979
- symfony/ux#2607

Commits
-------

4d2ccf4 Fix md formatting
f819aed [PropertyInfo] Deprecate Type
symfony-splitter pushed a commit to symfony/doctrine-bridge that referenced this pull request Mar 22, 2025
This PR was merged into the 7.3 branch.

Discussion
----------

[PropertyInfo] Deprecate `Type`

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

A new attempt to symfony/symfony#53160, now that `symfony/type-info` is not experimental anymore.

Deprecates:
- `Type` class in favor of the `Type` class of `symfony/type-info`
- `PropertyTypeExtractorInterface::getTypes()` in favor of the `PropertyTypeExtractorInterface::getType()` method
- `ConstructorArgumentTypeExtractorInterface::getTypesFromConstructor()` in favor of the `ConstructorArgumentTypeExtractorInterface::getTypeFromConstructor()` method

The work for upgrading dependent packages has begun already:
- api-platform/core#6979
- symfony/ux#2607

Commits
-------

4d2ccf4ac94 Fix md formatting
f819aed8d13 [PropertyInfo] Deprecate Type
@Kocal
Copy link
Member

Kocal commented May 27, 2025

@mtarld Sorry to bring this up again, but are we sure it's not possible at all to ship this PR for 2.x?
Like, if we remove the symfony/type-info from require-dev, and tweak the CI to install it when PHP >=8.2?

@mtarld
Copy link
Contributor Author

mtarld commented May 27, 2025

The UX LiveComponent is requiring a too low PHP version (lower than TypeInfo), which means that a composer install in low deps will fail. Therefore we can't have symfony/type-info in the composer.json file.

As discussed with @Kocal , the solution for 2.x will be:

  • Telling in the documentation to run composer require symfony/type-info in order to kill the deprecation (if they're using a lower version than 7.3, it won't be any deprecation).
  • Do the same in the CI.
  • Make sure that the code work for either PropertyInfo and TypeInfo.

Then for 3.x:

  • Remove the BC layer for PropertyInfo's Type
  • Bump the minimum PHP version
  • Require symfony/type-info in composer.json

I'll update the PR in that way ASAP 🙂

@smnandre
Copy link
Member

Do the same in the CI.

I'd like to keep the CI testing the actual behaviour users would have with 8.1 🤷

@mtarld poke me if you want some hand there :)

@Kocal
Copy link
Member

Kocal commented May 28, 2025

I'd like to keep the CI testing the actual behaviour users would have with 8.1 🤷

It won't change for 8.1, since symfony/type-info is not installable, but it will change for higher jobs

@mtarld mtarld force-pushed the feat/type-info-type branch from 81650fd to 69cdc81 Compare May 28, 2025 09:49
@mtarld
Copy link
Contributor Author

mtarld commented May 28, 2025

Status: Needs Work

@carsonbot carsonbot added Status: Needs Work Additional work is needed and removed Status: Needs Review Needs to be reviewed labels May 28, 2025
@mtarld mtarld force-pushed the feat/type-info-type branch from 69cdc81 to 3ce901a Compare May 28, 2025 10:14
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed and removed Status: Needs Work Additional work is needed labels May 28, 2025
@Kocal Kocal force-pushed the feat/type-info-type branch 10 times, most recently from 2e5126f to 0b25cab Compare May 29, 2025 10:01
@Kocal Kocal changed the title [LiveComponent] Fix PropertyTypeExtractorInterface::getTypes() deprecation, use TypeInfo Type [LiveComponent] Fix PropertyTypeExtractorInterface::getTypes() deprecation, use TypeInfo ^7.2 Type May 29, 2025
@Kocal Kocal force-pushed the feat/type-info-type branch from 0b25cab to b12ee10 Compare May 29, 2025 10:06
@Kocal
Copy link
Member

Kocal commented May 29, 2025

I rebased the PR but didn't squash my commits (if it can ease reviews), CI is full green (we can ignore fabbot).

After reviews, we can merge this PR for UX 2.x instead of 3.x, since the initial PR changed from "only use TypeInfo" to "use TypeInfo when possible" 🎉

Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

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

Wow, nice work @mtarld & @Kocal! I didn't think we'd see the CI green again in 2.x!

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels May 29, 2025
@smnandre
Copy link
Member

Wow so nice!

@Kocal
Copy link
Member

Kocal commented May 30, 2025

I've just pushed a new commit for documentation, about upgrading symfony/property-info:^7.1 to get rid of deprecations

EDIT: reverted, in fact it does not makes sense (the deprecation will be removed simply by upgrading the LiveComponent package).

@Kocal
Copy link
Member

Kocal commented May 30, 2025

After discussing with @mtarld, we've just increased symfony/type-info conflict to <7.2:

  • the previous conflict was wrong
  • a lot of breaking changes happened in TypeInfo 7.2 (since 7.1 was experimental)

@smnandre
Copy link
Member

🙇

Not enough time to make a deep review, but a huge thanks to you guys. 👏

Copy link
Member

@smnandre smnandre left a comment

Choose a reason for hiding this comment

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

❤️

@Kocal Kocal force-pushed the feat/type-info-type branch from e560fa5 to 7a08cce Compare May 30, 2025 18:43
@Kocal
Copy link
Member

Kocal commented May 30, 2025

Thanks Mathias. 🙌🏻

@Kocal Kocal merged commit 1f97e19 into symfony:2.x May 30, 2025
21 of 23 checks passed
@mtarld mtarld deleted the feat/type-info-type branch May 30, 2025 19:04
Kocal added a commit that referenced this pull request Jun 6, 2025
This PR was merged into the 2.x branch.

Discussion
----------

[LiveComponent] Fix conflict with `symfony/type-info`

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Docs?         | no <!-- required for new features -->
| Issues        | Fix #2827 <!-- 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).
-->

My bad, this conflict was supposed to be configured in #2607, but IDK where the commit went 🤷🏻

Commits
-------

0bb6d9f [LiveComponent] Fix conflict with `symfony/type-info`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New Feature LiveComponent Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[LiveComponent] PropertyInfoExtractor::getTypes() method is deprecated [LiveComponent] Transition to TypeInfo (fix 7.1 deprecations)
6 participants