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

[BUG]: Forms requires TagFactory #15847

Closed
niden opened this issue Dec 30, 2021 · 1 comment · Fixed by #15849
Closed

[BUG]: Forms requires TagFactory #15847

niden opened this issue Dec 30, 2021 · 1 comment · Fixed by #15849
Assignees
Labels
bug A bug report discussion Request for comments and discussion enhancement Enhancement to the framework status: high High

Comments

@niden
Copy link
Sponsor Member

niden commented Dec 30, 2021

The new implementation of HTML generators (TagFactory) is registered in the DI container using the name tag. It is required by the Forms component to start rendering components.

The constructor of Form accepts an entity and user data, stores that information and immediately calls initialize (with that data) if present in the child class.

Changing the signature of the constructor is going to break backwards compatibility for all applications that are being upgraded to v5. Since forms are a commonly used component, this will require a time consuming refactoring.

All the work that has been done in v4 and v5 was with the aim to no longer rely on a central DI container accessed as a singleton, rather have services injected into components by an new (future) container that will offer automatic injection of services.

However with the above scenario, this will not be feasible at least for Forms.

Steps to get the best balance for everyone:

  • v6 can introduce a new Html\Forms component that will use injected services in the constructor (TagFactory in particular)
  • v5 Forms (current) will:
    • Check if the TagFactory is defined (Element)
    • If not, check if there is a form attached and check the TagFactory there
    • If not present, use the static Di::getDefault() to resolve the tag service
    • If not present, instantiate a new TagFactory component

The above is not the best solution but it will help for this version, so that current applications will not need a huge refactoring effort.

@niden niden added bug A bug report status: unverified Unverified discussion Request for comments and discussion status: high High and removed status: unverified Unverified labels Dec 30, 2021
@niden niden self-assigned this Dec 30, 2021
@niden niden added this to Working on it in Phalcon Roadmap Dec 30, 2021
@niden niden mentioned this issue Jan 1, 2022
5 tasks
@niden niden linked a pull request Jan 1, 2022 that will close this issue
5 tasks
@niden niden added the enhancement Enhancement to the framework label Jan 1, 2022
@niden
Copy link
Sponsor Member Author

niden commented Jan 1, 2022

Fixed in #15849

@niden niden closed this as completed Jan 1, 2022
Phalcon Roadmap automation moved this from Working on it to Implemented Jan 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report discussion Request for comments and discussion enhancement Enhancement to the framework status: high High
Projects
Archived in project
Phalcon Roadmap
  
Implemented
Development

Successfully merging a pull request may close this issue.

1 participant