Skip to content

[Form] PRE_SUBMIT constant and value mismatch #24615

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

Closed
javiereguiluz opened this issue Oct 19, 2017 · 5 comments
Closed

[Form] PRE_SUBMIT constant and value mismatch #24615

javiereguiluz opened this issue Oct 19, 2017 · 5 comments
Labels

Comments

@javiereguiluz
Copy link
Member

Q A
Bug report? no
Feature request? no
BC Break report? no
RFC? no
Symfony version 4.0

In this tweet @deguif asked:

PRE_SUBMIT => 'form.pre_bind' in Symfony 4, is there any reason why the event name value didn't change?

Should we update the value of the constant?

@linaori
Copy link
Contributor

linaori commented Oct 19, 2017

We could add a second constant and deprecate the old one in 4.1 (or 3.4 if that's still allowed)

@stof
Copy link
Member

stof commented Oct 19, 2017

The PRE_SUBMIT constant cannot be deprecated. This is the name of the event we want (PRE_BIND was the old name in early 2.x).

@yceruto
Copy link
Member

yceruto commented Oct 19, 2017

The constant name was already changed in 3.0 (with previous deprecation since 2.3) but not its value, not sure why :/ . Most ppl 99.99% rely on the const name to listen, so maybe we can/should fix its value directly?

@stof
Copy link
Member

stof commented Oct 19, 2017

Yeah, I think we could change this in 4.0 without a big impact (btw, I think our BC policy even allows it for this case).

There is one case where people will use the event name rather than the constant: the kernel.event_listener tag (and recent versions can even use the constant in YAML files). But this won't be the case for this event, as form events are not dispatched in the main dispatcher anyway.

@yceruto
Copy link
Member

yceruto commented Oct 19, 2017

See #24631

symfony-splitter pushed a commit to symfony/form that referenced this issue Oct 19, 2017
…yceruto, javiereguiluz)

This PR was merged into the 4.0-dev branch.

Discussion
----------

[Form] Fix FormEvents::* constant and value matching

| Q             | A
| ------------- | ---
| Branch?       | 4.0
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes (ppl rely on const value directly, very weird)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony/symfony#24615
| License       | MIT
| Doc PR        | -

> symfony/symfony#24615 (comment) by @stof:
Yeah, I think we could change this in 4.0 without a big impact (btw, I think our BC policy even allows it for this case).
There is one case where people will use the event name rather than the constant: the kernel.event_listener tag (and recent versions can even use the constant in YAML files). But this won't be the case for this event, as form events are not dispatched in the main dispatcher anyway.

Commits
-------

944931af63 Minor reword
0ee856add1 Update UPGRADE-4.0.md
0fc2282fc2 fix the constant value to be consistent with the name
fabpot added a commit that referenced this issue Oct 19, 2017
…yceruto, javiereguiluz)

This PR was merged into the 4.0-dev branch.

Discussion
----------

[Form] Fix FormEvents::* constant and value matching

| Q             | A
| ------------- | ---
| Branch?       | 4.0
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | yes (ppl rely on const value directly, very weird)
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24615
| License       | MIT
| Doc PR        | -

> #24615 (comment) by @stof:
Yeah, I think we could change this in 4.0 without a big impact (btw, I think our BC policy even allows it for this case).
There is one case where people will use the event name rather than the constant: the kernel.event_listener tag (and recent versions can even use the constant in YAML files). But this won't be the case for this event, as form events are not dispatched in the main dispatcher anyway.

Commits
-------

944931a Minor reword
0ee856a Update UPGRADE-4.0.md
0fc2282 fix the constant value to be consistent with the name
@fabpot fabpot closed this as completed Oct 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants