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

Add Model.initialize event and rework how Component::initialize() works #4823

Merged
merged 4 commits into from
Oct 10, 2014

Conversation

markstory
Copy link
Member

This is a possibly controversial change that solves #4796 and an annoying difference around components vs. everything else.

New Shiny

  • New Model.initialize event that is fired after the initialize() method, just like in controllers.
  • Component::initialize() works like the other initialize methods. This also solves a long standing issue around sub components not getting their initialize method invoked.
  • Component::beforeFilter() added.
  • Controllers always have a request/response now.

Breaking changes

  • Component::intialize() is no longer an event listener.

This event follows the same pattern as Controller.initialize where it is
fired *after* the initialize() method and after other subscribers have
been attached. The initialize() method is *not* an event subscriber as
it is intended to be a post-constructor hook method like
Controller::initialize().

This will likely result in Component::initialize() being modified to be
conformant. There have been a number of reported issues in the past
around Component::initialize() not being called. It would be nice to put
those issues to rest and have consistency across the framework.
Make Component::initialize() consistent with Controller and Table, this
makes the framework easier to understand as methods with the same name
work consistently everywhere. While I'm not totally sure about what to
do with the Controller.initialize event hook in components, I feel this
change is a good one even though it breaks existing API compatibility.

The primary use of this event hook was RequestHandler which has been
updated to not rely on the event hook. I've also ensure that
Controllers always have a request and response object as that was
causing me some grief and logically makes sense.
This method is an event listener that listens to the
Controller.initialize event. I decided to copy the controller method
name so there was some semblance of consistency. This may be a bad idea
though.
@ionas
Copy link
Contributor

ionas commented Oct 6, 2014

👍 for this great consistency feature :)

@ADmad
Copy link
Member

ADmad commented Oct 7, 2014

By default there are no callbacks listening for Model.initialize right?

@robertpustulka
Copy link
Member

Great!

But I still don't quite get, why do we need two events dispatched one after another in Controller, that is Controller.initialize and Controller.startup. That is I realize that may be important for compatibility, but I guess old components are already broken with current changes :)

@markstory
Copy link
Member Author

@ADmad Correct.

@markstory
Copy link
Member Author

@robertpustulka Its mostly to afford a component to run code before/after Controller::beforeFilter().

@ravage84
Copy link
Member

ravage84 commented Oct 8, 2014

👍 Sounds reasonable

ADmad added a commit that referenced this pull request Oct 10, 2014
Add Model.initialize event and rework how Component::initialize() works
@ADmad ADmad merged commit 060a41c into 3.0 Oct 10, 2014
@ADmad ADmad deleted the issue-4796 branch October 10, 2014 18:51
@ADmad
Copy link
Member

ADmad commented Oct 10, 2014

Docs will need updating :)

@markstory
Copy link
Member Author

I can update the docs today.

dakota pushed a commit to dakota/crud that referenced this pull request Oct 13, 2014
Component::initialize() has been updated in CakePHP 3 to match other initialize() methods.
As per cakephp/cakephp#4823
dakota pushed a commit to dakota/search that referenced this pull request Oct 13, 2014
Component::initialize() has been updated in CakePHP 3 to match other initialize() methods.
As per cakephp/cakephp#4823
dakota pushed a commit to dakota/search that referenced this pull request Oct 13, 2014
Component::Initialize has moved to earlier in the bootstrap process in order to match other objects.
See cakephp/cakephp#4823
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants