Skip to content
This repository has been archived by the owner on Jan 21, 2020. It is now read-only.

Implement RouteResultObserverInterface #9

Merged

Conversation

weierophinney
Copy link
Member

This patch updates the UrlHelper to implement the new RouteResultObserverInterface.

Additionally, it provides a delegator factory for registering the UrlHelper as a RouteResult observer to the Application instance, via the following:

  • Creates a UrlHelperFactory for creating the UrlHelper, to register under the service name Zend\Expressive\ZendView\UrlHelper in the application container. (This simplifies usage of the delegator factory).
  • Updates the ZendViewRendererFactory to test for, and pull when present, the Zend\Expressive\ZendView\UrlHelper to create the URL helper.
  • Provides ApplicationUrlDelegatorFactory for injecting the UrlHelper as a RouteResult observer to the application instance; to do this, it checks to see if the Zend\Expressive\ZendView\UrlHelper is present, and, if so, uses that instance. (This minimizes the number of objects present in the dependency graph for cases when no template renderer is required.)

Updates UrlHelper to implement RouteResultObserverInterface.
This patch does the following:

- Creates a UrlHelperFactory for creating the UrlHelper, to register
  under the service name `Zend\Expressive\ZendView\UrlHelper` in the
  application container. (This simplifies usage of the delegator factory).
- Updates the ZendViewRendererFactory to test for, and pull when
  present, the `Zend\Expressive\ZendView\UrlHelper` to create the URL
  helper.
- Provides `ApplicationUrlDelegatorFactory` for injecting the
  `UrlHelper` as a RouteResult observer to the application instance; to
  do this, it checks to see if the `Zend\Expressive\ZendView\UrlHelper`
  is present, and, if so, uses that instance. (This minimizes the number
  of objects present in the dependency graph for cases when no template
  renderer is required.)
@weierophinney weierophinney added this to the 0.3.0 milestone Nov 30, 2015
@weierophinney
Copy link
Member Author

Composer issues. Locally, when I do a fresh install, I get dev-master of Expressive; when Travis runs, it's not getting the update, and thus unable to find the interface. Still searching for a solution.

weierophinney added a commit to weierophinney/zend-expressive-skeleton that referenced this pull request Nov 30, 2015
As of zendframework/zend-expressive#200, a new interface,
`Zend\Expressive\Router\RouteResultObserverInterface` exists to allow notifying
instances of the calculated `RouteResult`; this is particularly of interest to
URI generators.

With zendframework/zend-expressive-zendviewrenderer#9, the zend-view integration
provides an observer via the `UrlHelper` it ships, and allows you to attach it
to the application via a delegator factory.

This patch defines the delegator factory in the template configuration for
zend-view, and also details how you would accomplish similar functionality with
Pimple.
@harikt
Copy link
Contributor

harikt commented Dec 1, 2015

@weierophinney I think there is nothing wrong with composer. See

"zendframework/zend-expressive": "^1.0@rc",
it is looking for rc. And I believe expressive is not yet released another version with the Interface. Either you may want to change it to add minimum-stability flag of dev to the composer.json . So that the root of the package can install dev versions.

@weierophinney
Copy link
Member Author

@harikt You're looking at the master branch, not this patch; this patch changes the composer.json to use ~1.0.0-dev@dev:

@@ -19,7 +19,7 @@
"require": {
"php": ">=5.5",
"container-interop/container-interop": "^1.1",
"zendframework/zend-expressive": "^1.0@rc",
"zendframework/zend-expressive": "~1.0.0-dev@dev",
Copy link
Contributor

Choose a reason for hiding this comment

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

my assumption is this may be problematic when zend-expressive need to download zendviewrenderer. But I am not exactly sure if this will work without minimum-stability flag in expressive. How about you add minimum-stability dev to the composer.json and keep the rest in tact ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It will essentially grab dev-master always. I think instead I'm going to tag the zend-expressive repo, and then update this one to the latest tag.

I understand why everyone wanted to separate out the implementations, but while we're not stable, this is a nightmare to maintain the dependencies!

Copy link
Contributor

Choose a reason for hiding this comment

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

It will essentially grab dev-master always

Yes only for this repo. But you can change later :) .

and I can understand the pain. There should be some sort of scripts to do automatic releases and tagging when we have many repos.

@harikt
Copy link
Contributor

harikt commented Dec 1, 2015

Thank you. I didn't looked that.

@weierophinney weierophinney merged commit 057d2b8 into zendframework:master Dec 2, 2015
weierophinney added a commit that referenced this pull request Dec 2, 2015
weierophinney added a commit that referenced this pull request Dec 2, 2015
@weierophinney weierophinney deleted the hotfix/routeresult-observer branch December 2, 2015 20:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants