Skip to content

[String] snake() method changes in 7.1.2 breaks symfony/ux-translator #57612

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
Closed
jmsche opened this issue Jul 1, 2024 · 8 comments
Closed

[String] snake() method changes in 7.1.2 breaks symfony/ux-translator #57612

jmsche opened this issue Jul 1, 2024 · 8 comments
Labels
Bug Status: Needs Review String

Comments

@jmsche
Copy link
Contributor

jmsche commented Jul 1, 2024

Symfony version(s) affected

7.1.2

Description

After upgrading to Symfony 7.1.2 this morning, Symfony UX Translator won't compile anymore with Webpack.

Here is part of the error:

Syntax Error: Missing initializer in const declaration. (1:39)

> 1 | export const THIS_VALUE_SHOULD_BE_FALSE. = {"id":"This value should be false.","translations":{"validators":{"en":"This value should be false.","fr":"Cette valeur doit \u00eatre fausse.","nl":"Deze waarde moet onwaar zijn."}}};

Downgrading symfony/string to 7.1.1 solves the issue.

How to reproduce

echo (new UnicodeString('This value should be false.'))->snake()->toString();
// symfony/string 7.1.1: this_value_should_be_false
// symfony/string 7.1.2: this_value_should_be_false.

Possible Solution

The bug has been introduced with this PR: #57497
Reverting this solves the issue.

I guess it is a BC break?

Additional Context

No response
@jmsche jmsche added the Bug label Jul 1, 2024
@derrabus derrabus mentioned this issue Jul 1, 2024
Merged
@derrabus
Copy link
Member

derrabus commented Jul 1, 2024

cc @arczinosek

@jmsche
Copy link
Contributor Author

jmsche commented Jul 1, 2024

FYI the patch included in #57594 does not seem to fix the issue.

@jmsche jmsche mentioned this issue Jul 1, 2024
Closed
@xabbuh xabbuh mentioned this issue Jul 1, 2024
Closed
@xabbuh
Copy link
Member

xabbuh commented Jul 1, 2024

Can you please try #57615?

@kingjia90 kingjia90 mentioned this issue Jul 1, 2024
Merged
@jmsche
Copy link
Contributor Author

jmsche commented Jul 1, 2024 •
edited
Loading

@xabbuh The patch works... For the string I wrote as example above 😅

For translations with built translation keys it's still a huge BC break...

Let's just take this example:

form:
    none: No item selected

UX Translator previously created a key for the translation, called FORM_NONE (I guess String somehow replaced the link between the two keys with an underscore), now the translation seems to be created with the FORMNONE key.

Edit: I'm writing this simple example, but in my case it's more than 300 translations that are not generated correctly anymore.

@bobvandevijver
Copy link
Contributor

The snake update broke cache tags for me, but the fix in #57615 does solve my issue. For completeness, these are the output I recorded with the different versions.

Version Input Output
6.4.8 /ContainerOxPwblf\ResearchInstituteServiceGhostAad4a5e container_ox_pwblf_research_institute_service_ghost_aad4a5e
6.4.9 /ContainerOxPwblf\ResearchInstituteServiceGhostAad4a5e /container_ox_pwblf\research_institute_service_ghost_aad4a5e

With the proposed fix the behaviour of 6.4.8 returns for us.

@xabbuh
Copy link
Member

xabbuh commented Jul 1, 2024

@jmsche Can you debug what input is passed to the snake() method when the returned value is FORM_NONE? With that info I can take another look into what else needs to be fixed.

@jmsche
Copy link
Contributor Author

jmsche commented Jul 1, 2024

@xabbuh It's transforming form.none to FORM_NONE.

@nicolas-grekas nicolas-grekas mentioned this issue Jul 1, 2024
Merged
fabpot added a commit that referenced this issue Jul 1, 2024
@fabpot
bug #57616 [String] Revert "Fixed u()->snake(), b()->snake() and s()-…
e3625f2 
…>snake() methods" (nicolas-grekas)

This PR was squashed before being merged into the 5.4 branch.

Discussion
----------

[String] Revert "Fixed u()->snake(), b()->snake() and s()->snake() methods"

| Q             | A
| ------------- | ---
| Branch?       | 5.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #57612
| License       | MIT

This PR reverts #57497 for BC reasons, but keeps the test cases we added in the process. Those test cases allowed to spot a real issue where the ascii and unicode implementations didn't agree on the resulting camel case for `SYMFONY IS GREAT`. Both implementations now result in `SYMFONYISGREAT` (likely introduced in #47423).

Commits
-------

6397c38 [String] Revert "Fixed u()->snake(), b()->snake() and s()->snake() methods"
@fabpot fabpot closed this as completed Jul 1, 2024
@xabbuh xabbuh mentioned this issue Jul 3, 2024
Merged
fabpot added a commit that referenced this issue Jul 6, 2024
@fabpot
minor #57628 [String] test: kebab-case to snake_case (wmouwen)
737d519 
This PR was merged into the 5.4 branch.

Discussion
----------

[String] test: kebab-case to snake_case

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

In Symfony 7.1.1, `kebab-case` strings casted to `snake_case` properly. The changes made in 7.1.2 broke this functionality and kept it `kebab-case`.

It would be nice to have a `kebab-case` test added to the list to clearly define the expected behavior.

Relates to: #57497, #57612, #57616

Commits
-------

a6d0f3b test: kebab-case to snake_case
@aleho aleho mentioned this issue Jul 6, 2024
Merged
@jkgroupe
Copy link

jkgroupe commented Jul 8, 2024

Same issue with the version symfony/string:6.4.9. I downgraded it to 6.4.8

@derrabus derrabus mentioned this issue Jul 9, 2024
Closed
@kevinpapst kevinpapst mentioned this issue Jul 19, 2024
Closed
@antten antten mentioned this issue Jul 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Status: Needs Review String
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@fabpot @derrabus @bobvandevijver @xabbuh @jmsche @jkgroupe @carsonbot