Skip to content
This repository was archived by the owner on Dec 14, 2018. It is now read-only.

Adding support for flush points in Razor pages #1048

Closed
wants to merge 3 commits into from
Closed

Conversation

pranavkm
Copy link
Contributor

Fixes #1042

@pranavkm
Copy link
Contributor Author

cc @yishaigalatzer

<strong>Marketing:</strong> <a href="mailto:[email protected]">[email protected]</a>
</address>

@* Todo replace with await when we have support for async sections *@
Copy link
Contributor

Choose a reason for hiding this comment

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

suggest placing this at the top of the section to make delay more visible

@pranavkm pranavkm changed the title Adding support for flush points in Razor pages [Design]Adding support for flush points in Razor pages Aug 21, 2014
@pranavkm pranavkm changed the title [Design]Adding support for flush points in Razor pages [Design] Adding support for flush points in Razor pages Aug 21, 2014

// Assert
var body = await result.HttpContext.Response.ReadBodyAsStringAsync();
Assert.Equal(expected, body.Trim());
Copy link
Contributor

Choose a reason for hiding this comment

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

what about this test demonstrates the flush point is anything but a noop?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of it was to simply verify if the page executes. I think the right test would be to check if we get the appropriate number of chunks with the contents split for each invocation of Flush. The API that we have on client isn't extensive enough to allow this. I an speak to @javiercn to figure out if there's some way to get to the raw results

@pranavkm pranavkm force-pushed the FlushPoints branch 3 times, most recently from 923094c to 746f219 Compare August 23, 2014 01:18
@pranavkm pranavkm force-pushed the FlushPoints branch 2 times, most recently from b0fb438 to 08eac45 Compare August 27, 2014 00:52
@pranavkm
Copy link
Contributor Author

  • Moved FlushPoint behavior to RazorTextWriter.
  • Updated the functional test to actually verify chunked responses.

@pranavkm pranavkm changed the title [Design] Adding support for flush points in Razor pages Adding support for flush points in Razor pages Aug 28, 2014
@pranavkm
Copy link
Contributor Author

Updated.

FYI @yishaigalatzer nested sections don't work (and did not in Mvc 5). Regardless, this approach would allow it but we can't write a functional test for it.

@nikmd23
Copy link

nikmd23 commented Aug 29, 2014

I'm pretty excited about this PR! To be fair I petitioned for this feature pretty hard...

One word of warning, that I found via the library I hacked toget to emulate this (called CourtesyFlush): in MVC 5, Html.AntiForgeryToken() writes to the HTTP headers to set a cookie from the view. This means that AntiForgeryToken, with its current implementation, cannot work after a flush.

I had to reimplement AntiForgeryToken for my library to work and provide an option to set the cookie at the flush point.

Perhaps you can do something more clever, or at least allow for FlushAsync(includeAntiForgeryToken: true)

Let me know if this makes sense or not.

@yishaigalatzer
Copy link
Contributor

@nikmd23 that is a great reminder. Opened a followup item #1079

@NTaylorMullen @DamianEdwards this is something we need to account for in the form taghelper as well.

CC @GrabYourPitchforks @harshgMSFT

@yishaigalatzer
Copy link
Contributor

:shipit:

@pranavkm pranavkm closed this Aug 29, 2014
@pranavkm pranavkm deleted the FlushPoints branch August 29, 2014 16:36
@nikmd23 nikmd23 mentioned this pull request Sep 9, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants