-
-
Notifications
You must be signed in to change notification settings - Fork 188
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
!!! FEATURE: ViewInterface
returns PSR StreamInterface
#3286
!!! FEATURE: ViewInterface
returns PSR StreamInterface
#3286
Conversation
@kitsunet That might be in conjunction with #3232 an actual step to make the controllerContext obsolete? But to still support martins use case and have the final readonly class ActionMessages
{
private function __construct(
public ActionRequest $request,
public ActionResponse $response
) {
}
public static function create(ActionRequest $request, ActionResponse $response)
{
return new self($request, $response);
}
} This is the secret passing logic where we make the $this->view->assign('actionMessages', ActionMessages::create($this->request, $this->response));
if (method_exists($this->view, 'setControllerContext')) {
$this->view->setControllerContext($this->controllerContext);
} One could say that nothing changed but i think the Of course it would be ideal to not have to pass the pre- |
yes that would be the goal. To only pass the request but not the Response outside of the view and its possible!!! In this pr i found a way to make the neos fusion view work with plugins and fusion forms despite having no controller context at hand. neos/neos-development-collection#4856 It works by creating a dummy controller context in the view and later rendering out that response from the view. That gives us the possibility to actually decouple views and controllers a lot. Simply by using the feature to render a psr response from a view. The same i did in a much smaller scope for the json view, so that the content type is set automatically. My prior |
ViewInterface
independent of ControllerContext
Initially introduced the option expected an actual instance of `FusionGlobals`. This is a flawed design as only primitives can be set for example via `Views.yaml` Additionally, the `FusionView` is forward compatible for this change neos/flow-development-collection#3286 The `fusionGlobals` must not be used for setting the request but rather ``` $view->assign('request"', $this->request) ```
The Neos `FusionView` is forward compatible for this change neos/flow-development-collection#3286 Assign should be used for setting the request: ``` $view->assign('request"', $this->request) ```
use Neos\Flow\Persistence\PersistenceManagerInterface; | ||
use Neos\Utility\ObjectAccess; | ||
use Neos\Utility\TypeHandling; | ||
use Psr\Http\Message\ResponseInterface; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deprecate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that we deprecated the json view ... what is our answer to code like this? :O
/**
* @var array
*/
protected $viewFormatToObjectNameMap = [
'html' => FusionView::class,
'json' => JsonView::class,
];
do we need a new simple json view? :P
cc @kitsunet
ViewInterface
independent of ControllerContext
ViewInterface
ViewInterface
ViewInterface
without controller context
3ff7973
to
7b568f6
Compare
676ea9a
to
7b04e7b
Compare
Initially introduced the option expected an actual instance of `FusionGlobals`. This is a flawed design as only primitives can be set for example via `Views.yaml` Additionally, the `FusionView` is forward compatible for this change neos/flow-development-collection#3286 The `fusionGlobals` must not be used for setting the request but rather ``` $view->assign('request"', $this->request) ```
834b104
to
aa7a41e
Compare
Neos.Flow/Tests/Functional/Mvc/ViewsConfiguration/Fixtures/TemplateView.php
Show resolved
Hide resolved
@@ -26,53 +25,45 @@ interface ViewInterface | |||
/** | |||
* Sets the current controller context | |||
* | |||
* @deprecated if you absolutely need access to the current request please assign a variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this even show in IDEs with the function commented out? bit nasty I guess 🤷
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documenting this via
@method setControllerContext(ControllerContext $controllerContext) @deprecated
on the view-interface will not have the desired effect of annotating it as deprecated but would still allow goto clicking.
In the end all views extend the AbstractView which still contains a definition for setControllerContext
(but deprecated) so i guess its fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the more important headline this PR should have is View returns Stream, because that will hit many peoples feet updating to this. I think it's a good direction, minor comments, probably overlooked osmething, probably something needs fixing in the future, but we should do it.
ViewInterface
without controller contextViewInterface
returns PSR StreamInterface
{ | ||
$this->actionController = $this->getAccessibleMock(ActionController::class, ['resolveActionMethodName', 'initializeActionMethodArguments', 'initializeActionMethodValidators', 'resolveView', 'callActionMethod']); | ||
$this->actionController->method('resolveActionMethodName')->willReturn('indexAction'); | ||
|
||
$this->inject($this->actionController, 'objectManager', $this->mockObjectManager); | ||
$this->inject($this->actionController, 'controllerContext', $this->mockControllerContext); | ||
$this->inject($this->actionController, 'request', $this->mockRequest); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last question, do we want to use request
as naming here or preserve that slot for a pretty psr request, despite that in Fusion request is an ActionRequest (which will not change anytime?)
Do we all agree to keep the ActionRequest for some time and have it hold all the context, like matched controller, base uri and routing parameters?
…text" This reverts commit 2717a8c.
Initially `canRender` was introduced to allow special logic in a controller to determine the correct view out of multiple options. neos@cb8fa24 But with neos@d69bcfe the canRender() checks were removed and leaves the method with zero usages and no _real_ implementation.
…face` Please adjust your view to return a `StreamInterface` instead of a stream. If you need a string at call site, please use `->getContent()` instead.
aa7a41e
to
524f61e
Compare
Initially introduced the option expected an actual instance of `FusionGlobals`. This is a flawed design as only primitives can be set for example via `Views.yaml` Additionally, the `FusionView` is forward compatible for this change neos/flow-development-collection#3286 The `fusionGlobals` must not be used for setting the request but rather ``` $view->assign('request"', $this->request) ```
Initially introduced the option expected an actual instance of `FusionGlobals`. This is a flawed design as only primitives can be set for example via `Views.yaml` Additionally, the `FusionView` is forward compatible for this change neos/flow-development-collection#3286 The `fusionGlobals` must not be used for setting the request but rather ``` $view->assign('request"', $this->request) ```
Neos adjustments neos/neos-development-collection#4856
ControllerContext
ViewInterface::setControllerContext
is not part of the interface anymore and will only be called on demandActionRequest
if necessary can be accessed via the variable "request" (like "settings")ViewInterface::canRender
was required for fallbackviews which have been removed long ago, and so this artefact will be removed as well.ResponseInterface
or aStreamInterface
. Simple strings must be wrapped in a psr stream! (seeStreamFactoryTrait::createStream
)Related to #3232