Skip to content

[Form] Deprecate FormEvent::setData() for events that do not allow it #51043

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 21, 2023
Merged

[Form] Deprecate FormEvent::setData() for events that do not allow it #51043

merged 1 commit into from
Jul 21, 2023

Conversation

HeahDude
Copy link
Contributor

Q A
Branch? 6.4
Bug fix? no
New feature? no
Deprecations? yes
Tickets ~
License MIT
Doc PR TODO

Now that specific classes exist for each event, we can improve DX by forbidding to call FormEvent::setData() where it does not make sense.

@HeahDude HeahDude requested review from xabbuh and yceruto as code owners July 20, 2023 10:48
@carsonbot carsonbot added this to the 6.4 milestone Jul 20, 2023
@HeahDude HeahDude force-pushed the deprecate-form-event-set-data branch from 22b8b28 to 47e5017 Compare July 20, 2023 10:56
xabbuh
xabbuh reviewed Jul 20, 2023
View reviewed changes
@HeahDude HeahDude force-pushed the deprecate-form-event-set-data branch from 47e5017 to 6e11231 Compare July 20, 2023 12:30
xabbuh
xabbuh reviewed Jul 20, 2023
View reviewed changes
xabbuh
xabbuh reviewed Jul 20, 2023
View reviewed changes
@HeahDude HeahDude force-pushed the deprecate-form-event-set-data branch from 6e11231 to c081789 Compare July 20, 2023 13:34
xabbuh
xabbuh requested changes Jul 20, 2023
View reviewed changes
@HeahDude HeahDude force-pushed the deprecate-form-event-set-data branch from c081789 to a8d70e2 Compare July 20, 2023 14:34
nicolas-grekas
nicolas-grekas reviewed Jul 21, 2023
View reviewed changes
@HeahDude HeahDude force-pushed the deprecate-form-event-set-data branch 2 times, most recently from 1650c82 to 7c27e6a Compare July 21, 2023 08:23
xabbuh
xabbuh reviewed Jul 21, 2023
View reviewed changes
xabbuh
xabbuh reviewed Jul 21, 2023
View reviewed changes
@HeahDude HeahDude force-pushed the deprecate-form-event-set-data branch from 7c27e6a to 0d318d0 Compare July 21, 2023 09:14
@nicolas-grekas
Copy link
Member

Thank you @HeahDude.

@nicolas-grekas nicolas-grekas merged commit 503a7b3 into symfony:6.4 Jul 21, 2023
@nicolas-grekas nicolas-grekas mentioned this pull request Jul 21, 2023
Closed
@HeahDude HeahDude deleted the deprecate-form-event-set-data branch July 21, 2023 12:01
This was referenced Oct 21, 2023
Merged
Merged
@ThomasLandauer ThomasLandauer mentioned this pull request Nov 30, 2023
Closed
@anyt
Copy link

anyt commented Jan 2, 2024

Hi guys,
As I've spent hours to get into the fact that the method now does literally nothing, while the business logic expects it to update data as it was in 5.4, I think it worth sharing.

I expect this to trigger exception in 6.4 as well, or trigger a deprecation and work as before. Otherwise it's a logical backward compatibility break that is hard to find because it only triggers a deprecation.

From my perspective the deprecation is about using an obsolete way to do the thing, while here the method just does nothing and I have no choice except using the new approach to make it work back.

@fancyweb
Copy link
Contributor

fancyweb commented Jan 2, 2024

Hello, please open a new issue. At first glance, I think we forgot to call parent::setData() after triggering the deprecation?

@anyt
Copy link

anyt commented Jan 2, 2024

Created, #53364. Thank you.

@fancyweb fancyweb mentioned this pull request Jan 3, 2024
Merged
xabbuh added a commit that referenced this pull request Jan 4, 2024
@xabbuh
bug #53381 [Form] Fix assigning data in PostSetDataEvent and `PostS…
2e73037 
…ubmitEvent` (fancyweb)

This PR was merged into the 6.4 branch.

Discussion
----------

[Form] Fix assigning data in `PostSetDataEvent` and `PostSubmitEvent`

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

Ref #51043 where we changed the behavior on 6.4 since we don't assign the data property anymore.

cc `@HeahDude`

Commits
-------

61d71c4 [Form] Fix assigning data in PostSetDataEvent and PostSubmitEvent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas left review comments

@xabbuh xabbuh xabbuh approved these changes

@derrabus derrabus derrabus approved these changes

@yceruto yceruto yceruto approved these changes

Assignees
No one assigned
Labels
Deprecation Form Status: Reviewed
Projects
None yet
Milestone
6.4
Development

Successfully merging this pull request may close these issues.
8 participants
@HeahDude @nicolas-grekas @anyt @fancyweb @derrabus @xabbuh @yceruto @carsonbot