Skip to content

Conversation

@hrach
Copy link
Contributor

@hrach hrach commented Jan 19, 2016

The json output isn't something special what shouldn't be cached at all. If disabling of cache is needed, this should be handled by Nette Presenter itself.

It's probably BC Break.

@Majkl578
Copy link
Contributor

👍 Disabling caching doesn't make sense.

@fprochazka
Copy link
Contributor

Is there a way to override this? Or you just couldn't use the response object if you wanned it to be cached?

Maybe an option would be better than removing it completely.

@enumag
Copy link
Contributor

enumag commented Jan 19, 2016

👍 Makes sence.

@dg
Copy link
Member

dg commented Jan 19, 2016

Can you change json response to JsonResponse in commit message?

@hrach
Copy link
Contributor Author

hrach commented Jan 20, 2016

Repushed with BC break info.

@enumag
Copy link
Contributor

enumag commented Jan 20, 2016

Maybe we should add $expiration parameter to Presenter::sendJson();? Not sure about what the default value should be though.

@Majkl578
Copy link
Contributor

Why? You can configure it directly through Http\Response object.

@hrach
Copy link
Contributor Author

hrach commented Jan 20, 2016

I don't see the added valude, only the added complexity of another parameter. Feel free to change the expiration in the Response instance.

@enumag
Copy link
Contributor

enumag commented Jan 20, 2016

Well to achieve the original behavior I'll have to replace all of my occurences of sendJson with

$response = new JsonResponse($data);
$response->setExpiration(false);
$this->sendResponse($response);

The parameter would simplify it pretty well.

@Majkl578
Copy link
Contributor

Or you can just use:

$this->getHttpResponse()->setExpiration(FALSE);
$this->sendJson(...);

@dg dg force-pushed the master branch 5 times, most recently from 18f376d to 3fe619f Compare January 22, 2016 03:07
dg added a commit that referenced this pull request Jan 29, 2016
json response: removed automatic cache disabling
@dg dg merged commit 3510d2c into nette:master Jan 29, 2016
@hrach hrach deleted the patch-1 branch January 29, 2016 18:10
@hrach
Copy link
Contributor Author

hrach commented Jan 29, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants