feat(vercel): streaming

[lilnasy]

Changes

• Before: https://astro-blog-template-i8g8rijty-lilnasy.vercel.app/streaming
• After: https://astro-blog-template-o79sdirk5-lilnasy.vercel.app/streaming
• https://vercel.com/docs/build-output-api/v3/primitives#base-config

Testing

Added a fixture.

Docs

Changeset only.

[changeset-bot]

🦋 Changeset detected

Latest commit: b05a2ec

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[natemoo-re]

Oh I had no idea we didn't already have this enabled! Seems like it should default to true, yeah?

[lilnasy]

Yeah, but that's a major change isn't it. I think it could go as minor and opt-in for and we switch the default whenever we feel like releasing a major version.

[natemoo-re]

Ah yes, didn't consider that. Makes sense!

[natemoo-re]

LGTM! As I mentioned, this would make sense to enabled by default in the next major version of the adapter.

[natemoo-re]

I feel like @sarah11918 might have thoughts on the wording here!

[lilnasy]

I'm hoping lol

[sarah11918]

I want to see which direction the README takes before bringing that over here! Don't let me forget about this, though!

[sarah11918]

Suggested change

	Use this property to set whether the serverless function will stream the response to the browser.
	Determines whether or not the serverless function will stream its response to the browser, sending components as they are rendered.
	The default value is `false`. Set this value to `true` in your Vercel adapter configuration to enable streaming.

I'm assuming the default is false, if you have to set it to be true to take effect? I think it would be good to state that. Maybe something like above?

Also, to check:

1. In the caution just under here in docs: https://docs.astro.build/en/guides/server-side-rendering/#astrorequestmethod , and a little further down the page in the streaming section, it sounds like streaming is automatically used for SSR / output: 'server'. So at first I was confused to even see this PR! If this is not the case, then do the other docs need updating? Just trying to reconcile what's here vs on the existing SSR page.

2. That caution I linked to explains that because of streaming, you have to use features that modify the response headers only at the page level and not the component level (because the response headers may already be sent by the time some components render). Should a similar warning exist or be linked to from here re: enabling streaming? Is this a point you think might be important for someone to know, or potentially a breaking change in an existing project if they enable this?

[lilnasy]

I'm assuming the default is false

Effectively, yes, but it's not out default, it is Vercel's. So, I'm a bit hesitant to explicitly to document it on our site.

1. In this case it is the platform that interfered and made it all-at-once. I was not aware that Vercel did this, and neither was Nate. I just stumbled upon a suspicious Vercel reference doc and had to try it out to confirm. We have to explicitly tell Vercel to actually allow streaming.

2. No, the limitations apply the same no matter what the platform does once Astro is done. And there isn't too much of a breaking concern, just a formality in this case (when we release a major version to make it the default). Everything behaves the same other than the timing of the response parts.

[lilnasy]

Also, the suggestion looks good to me but it does make it inconsistent with descriptions of the other config options in the README.

[sarah11918]

OK, so thinking about this then! I'm looking back through earlier convo and noticing Nate thought it was true by default.

So, I agree, this actually does sound in keeping with what we expected and documented! No worries about those extra concerns!

Give me a few minutes more to think about updated wording.

[sarah11918]

Suggested change

	Determines whether or not the serverless function will stream its response to the browser, sending components as they are rendered.
	The default value is `false`. Set this value to `true` in your Vercel adapter configuration to enable streaming.
	Use this property to enable Astro's HTML streaming.

I kind of want something like this now, because the docs really do imply this happens by default. (And yes, this seems to be more consistent with the others.)

What do you think about something like this?

[lilnasy]

Hmm it won't be limited to HTML. Although the non-HTML cases would be where the user is creating their own streaming responses from scratch.

[sarah11918]

OK, so I'll tell you where my little stumbling blocks are and YOU propose what works! 😄

Grammatically:

• "the serverless function" sounds jarring out of place. I never like a "the" if we haven't defined something yet. If you can be more descriptive and avoid the implied context of a "the", that would go a long way.

Philosophically:

• I really don't like that regular docs say that we do HTML streaming and apparently, sometimes we don't! So, I'd be happier from a narrative standpoint if this default were set to true (can we do that? can we build it that way?) and then this configuration turns it OFF. (Note: even if this feature builds the ability to turn it on, maybe that doesn't have to be the way the feature is USED by the user?) Then the story matches the other adapters, and we can say that you CAN turn it off to do x, y, and z if we want to give use cases that are not... just how our stuff is supposed to work? I feel we can get away with being less define-y here in that case, and say you can remove and either not use, roll your own etc.

[lilnasy]

So, the serverless function is the particular one created when you deploy to Vercel, at least intended to be that one.

So, I'd be happier from a narrative standpoint if this default were set to true

I agree, it's a documented feature. We could probably make a major release to change the default very soon (but not right away), so that streaming is always the case. And I think it is a good idea for another major release after to remove the option altogether.

Then the story matches the other adapters

Netlify never streams either 😶

[lilnasy]

can we build it that way?
Yes :D

[lilnasy]

It is now built that way. Streaming with no configuration to opt-out, just the way it should be.

[sarah11918]

Netlify never streams either 😶

What even is happening?? The cake is a lie! 😅

So, I don't want anyone's project to break with this change, of course! Aligning this with docs is helpful for me, since that's the story we tell pretty clearly, but if we've been misinformed then we can change on our end, too.

[lilnasy]

Breaking is not a concern, just that the change is going to be more visible than what someone might expect from a minor or patch release. So a major release is important more because the user needs to be given notice about what's happening.

[bluwy]

Just checking, since this is marked a major, do users have to do something special to handle this change? If not and it would still work as usual, maybe it should be marked a minor instead?

[lilnasy]

@bluwy Yeah, that is a point of discussion. There's nothing for users to change, but I do think users need to be aware that it is happening, unlike with a patch or minor update. So I'm going with a major to be on the safe side.

[bluwy]

If it's only an under-the-hood change, I'd vote going with minors (or even a patch since we had always been streaming but never worked in Vercel). We had made larger internal changes in the past that are released under minor/patch. e.g refactoring the runtime to remove generators, and swapping shiki to shikiji.

[ematipico]

We aren't breaking anything, I would avoid a major. No need to alert the users.

Although it seems this is a default now...

I would like to avoid another functionPerRoute to be honest

[lilnasy]

@ematipico Opt-in to be enabled by default later in a major was the initial approach. Checking in with Matthew, the resolution was that major releases for adapters are not as precious as astro.

[sarah11918]

Left a suggestion for the changeset!

[sarah11918]

Suggested change

	This brings better performance to your visitors by showing them content as it is rendered. The browser can also start loading the required stylesheets and scripts much sooner, which ultimately results in faster full page loads.
	This brings better performance to your visitors by showing them content as it is rendered. The browser can also start loading the required stylesheets and scripts much sooner, which ultimately results in faster full page loads.
	Read more about [HTML streaming](https://docs.astro.build/en/guides/server-side-rendering/#html-streaming) in the Astro docs.

Up to you! Since there are warnings involved with streaming, it couldn't hurt to send to the new docs. BUT, since this doesn't break anything, not really needed anymore. (Was added when I was afraid that enabling this changed the Response headers behaviour)

NOTE: Depending on how quickly this merges, I have a PR that would add the anchor link above but currently the URL is only #streaming. If this is to merge soon, I can pre-emptively update existing docs to that link now, even before the new page is published.

[sarah11918]

Withdrawing this suggestion! With that docs page about to change, it's maybe a little too disruptive at this exact moment.

[matthewp]

I'm also comfortable with doing this as a minor if @lilnasy is. I don't have a strong preference, but felt that if it was between a config option or a major, that we might as well just do the major.

[lilnasy]

Switching to a minor release as there is consensus towards it.

[ematipico]

Looks good to me. I think I'm more comfortable with having this major because we are adding a new feature as a default. Technically it should be the same, but we have been so wrong in the past... 😆

I leave the final decision to you @lilnasy ! I trust your gut

[lilnasy]

I would've been more comfortable with major too. But having tried it out, I don't think there's room for unintended side effects to appear. I think minor works after weighing the paperwork against the feature.

[lilnasy]

!preview streaming (doesnt work, my branch is on a fork)