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

Helper optional renderer consistency #266

Merged
merged 12 commits into from
Feb 19, 2024

Conversation

oldpec
Copy link
Contributor

@oldpec oldpec commented Feb 1, 2024

Q A
Documentation no
Bugfix yes
BC Break no
New Feature no
RFC no
QA no

Description

Inconsistencies in the way different view helpers load plugins meant that while most helpers worked without a renderer being attached on $this->view, a few failed.

This PR brings the few cases where plugins weren't successfully created when $this->view wasn't populated into line with the others and includes unit tests that fail before the change.

  • Are you fixing a BC Break?

Arguably?
In some older versions of PHP (ie 7.4) some of these cases (eg FormSelect::getFormHiddenHelper()) would work (because $this->view being null only became a TypeError for method_exists() in PHP 8.0) and then stopped working on current PHP versions.
So these fixes would make code that worked for people under older versions of PHP continue to work as they uplift their PHP and Laminas versions.

There is a little background on:
https://discourse.laminas.dev/t/laminas-form-view-helper-retrieval-consistency/3591

Xerkus and others added 10 commits January 11, 2024 01:22
Signed-off-by: Aleksei Khudiakov <aleksey@xerkus.pro>
….x_MkCXLtZO

Merge release 3.19.1 into 3.20.x
Signed-off-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
…nder a hidden field

Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
…rmSelect if needed

Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
Copy link
Contributor

@Slamdunk Slamdunk left a comment

Choose a reason for hiding this comment

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

Thank you very much for fixing this bug 💪

Just minor improvements: only booleans should be put in if condition, and condition should be as narrow as possible to let unknown bugs emerge as soon as possible.

Then we can 🚢 it

src/View/Helper/AbstractFormDateSelect.php Outdated Show resolved Hide resolved
src/View/Helper/AbstractFormDateSelect.php Outdated Show resolved Hide resolved
src/View/Helper/FormMultiCheckbox.php Outdated Show resolved Hide resolved
src/View/Helper/FormSelect.php Outdated Show resolved Hide resolved
src/View/Helper/FormSelect.php Outdated Show resolved Hide resolved
Signed-off-by: Paul McGarry <paul@paulmcgarry.com>
@oldpec oldpec requested a review from Slamdunk February 14, 2024 02:14
@Slamdunk Slamdunk added the Bug Something isn't working label Feb 19, 2024
@Slamdunk Slamdunk added this to the 3.19.2 milestone Feb 19, 2024
@Slamdunk Slamdunk changed the base branch from 3.20.x to 3.19.x February 19, 2024 07:08
@Slamdunk Slamdunk merged commit f2ae01f into laminas:3.19.x Feb 19, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants