Skip to content
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

[v4] Fix compatibility with nette/forms 3.2.3 #108

Merged
merged 3 commits into from
May 14, 2024
Merged

[v4] Fix compatibility with nette/forms 3.2.3 #108

merged 3 commits into from
May 14, 2024

Conversation

jtojnar
Copy link
Collaborator

@jtojnar jtojnar commented May 7, 2024

nette/forms@e227d87 added an extra argument.

While at it, let’s also narrow the $values argument type to match Container::setValues.

This change remains compatible with nette/forms 3.2.2.

Backports #107

@radimvaculik
Copy link
Member

@jtojnar Hi, can you make PR green? Thanks.

@jtojnar
Copy link
Collaborator Author

jtojnar commented May 13, 2024

It is planned but I have not gotten around to it yet.

The PHPStan issue can be easily fixed by a cast – I verified that FileUpload cannot be returned with the arguments we pass to the method.

But I still need to debug the other test failure. I suspect it is caused by some BC break in Latte:

[Latte\CompileException] Unexpected tag {form}, did you mean {for}? (in '.../templates/group.latte' on line 1 at column 1)  

nette/forms@e227d87 added an extra argument.

While at it, let’s also narrow the `$values` argument type to match `Container::setValues`.

This change remains compatible with nette/forms 3.2.2.

(cherry picked from commits 1d9a356, 73f42f5)
@jtojnar jtojnar marked this pull request as ready for review May 14, 2024 01:28
@jtojnar
Copy link
Collaborator Author

jtojnar commented May 14, 2024

I have bisected the Latte failure to nette/application@1aba9da and resolved it in WebChemistry/testing-helpers#13.

nette/forms@0cd5069 added more types but that is overly broad for our usage pattern. We need to cast it to proper value until the return type is clarified upstream.

(cherry picked from commits 45cf2a8, bfe52eb)
`make coverage` would fail with:

    No code coverage driver available

Sounds like it might have been broken by xdebug 2 → 3 bump:
https://xdebug.org/docs/upgrade_guide#New-Concepts

Cargo culted from contributte/invoice@ca51236

(cherry picked from commit bfe52eb)
@jtojnar jtojnar merged commit 289316f into v4 May 14, 2024
7 checks passed
@jtojnar jtojnar deleted the backport branch May 14, 2024 01:40
@radimvaculik
Copy link
Member

@jtojnar Nice job, mate! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants