feat: add streaming arg to createApp()

[florian-lefebvre]

Description (required)

Related issues & labels (optional)

• Closes #
• Suggested label:

For Astro version: 6.x. See astro PR #15483.

[netlify]

✅ Deploy Preview for astro-docs-2 ready!

Name	Link
🔨 Latest commit	db61a90
🔍 Latest deploy log	https://app.netlify.com/projects/astro-docs-2/deploys/699c0d82510cb3000846edba
😎 Deploy Preview	https://deploy-preview-13273--astro-docs-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[astrobot-houston]

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File	Note
en/guides/upgrade-to/v5.mdx	Source changed, localizations will be marked as outdated.
en/guides/upgrade-to/v6.mdx	Localization changed, will be marked as complete.
en/reference/adapter-reference.mdx	Source changed, localizations will be marked as outdated.
en/reference/modules/astro-app.mdx	Localization added, will be marked as complete.
en/reference/modules/astro-static-paths.mdx	Localization changed, will be marked as complete.
ru/guides/upgrade-to/v5.mdx	Localization changed, will be marked as complete.

Warnings reference

Icon	Description
🔄️	The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

[sarah11918]

So tiny! 😅 If @ArmandPhilippot is happy, then I am happy!

[ArmandPhilippot]

Happy too! 😄

[sarah11918]

Neither of us are happy anymore! (Just to be clear 😂 )

[sarah11918]

Noting that now I think the changeset from the implementation PR has some text we can work with for creating a streaming entry!

[florian-lefebvre]

So just restating that I will merge the code PR because I need it for other stuff, but we don't need to rush to get this PR ready in time for release. No one uses createApp() apart from us atm

[sarah11918]

I am removing the merge-on-release tag because this is released now, and we are just getting around to the docs when we can!

[ArmandPhilippot]

I was looking at the source code in the core repo to figure out how new App() should be used... but given the recent changes you made it seems this is kinda deprecated for the end-users and the new method is to always use createApp(), right?

Are there any use-cases where using new App() directly is better? Or should this be removed from the docs at some point (i.e. next major)? And instead of:

The App constructor accepts a required SSR manifest argument, and optionally an argument to enable or disable streaming, defaulting to true.

We could say:

You can create a new App using createApp()... This gives you access to the following methods.

And the (updated) code snippet could be moved under createApp()?

[florian-lefebvre]

I think it makes sense to stop documenting astro/app and App in favor of just astro/app/entrypoint. I didn't accept any suggestions because I feel like this comment will change quite a few things

[sarah11918]

No need to accept suggestions, @florian-lefebvre ! Happy if you want to "start over" with what you think is most helpful on this page in general!

[florian-lefebvre]

I am not fully satisfied with the header structure of astro/app/entrypoint. The existing content for astro/app (eg. app.render()) already goes down to h6s

[ArmandPhilippot]

already goes down to h6s

Welcome to our nightmare. 😄

I agree this is a bit weird to have createApp() and all the methods at the same level... I don't have a good idea at the moment (I'll think about it) but I do have more questions...

Looking at the new structure, I realize this also means all the static methods are no longer relevant because we can't access them from createApp(), and we might need to update some descriptions. For example, RenderOptions.addCookieHeader currently links to App.getSetCookieFromResponse()...

So, are there any use cases for the following static methods (inherited from BaseApp)?

• App.getSetCookieFromResponse()
• App.validateForwardedHost()

Also, it seems the following static methods have been removed in v6 and so we'll need to remove them anyway:

• App.sanitizeHost()
• App.validateForwardedHeaders()

[florian-lefebvre]

I don't have context on these static methods, I'm not sure if they are deprecated or removed. Given that, it may be better to keep the astro/app heading after all

[ArmandPhilippot]

I was moving some sections, locally, to see how this could be organized without looking odd and to workaround the h6s. I wonder if it would make sense to have astro/app in the "Runtime API" section. I know this is only used with adapters but... we already have astro:static-paths in there.

Here's a preview of both TOC:

Adapter API TOC	astro/app TOC

In the Adapter API page I introduced a "Building adapter entrypoints" section in anticipation of the "Building a preview entrypoint" section. And "Passing build-time configuration" is probably for all entrypoints so we don't need to nest it under "Building a server entrypoint".

If we proceed this way:

• we probably need to think about a better name than "App API Reference" for the astro/app page
• even if we show App as an available import maybe we can just phrase the description to say this should be created with createApp()? Not ideal but with or without the reorganization I don't see how to separate the options from the methods returned... Unless we introduce some Options and Returned methods headings. Not possible at the moment because of the h6s though.
• (edit) there is also a path where we document App under createApp() with a heading like The App instance and we keep import from astro/app only if there is something else to document here

I also noticed there are a lot of available imports from astro/app we don't document. Do you know if we should Florian?

import {
  App,
  AppPipeline, // Probably not
  BaseApp,  // Probably not
  createConsoleLogger,
  deserializeManifest,
  deserializeRouteData,
  deserializeRouteInfo,
  fromRoutingStrategy,
  serializeRouteData,
  serializeRouteInfo,
  toRoutingStrategy,
  type RenderErrorOptions,
  type RenderOptions,
  type RoutesList,
} from "astro/app";

I haven't pushed those changes yet because there is more work to do (linking, check/improve the descriptions) and I'm not sure we want this. But, if you're not against that idea:

• if you think a preview could be useful to see how it looks before spending more time on it, I can create a draft PR tomorrow even if some links are broken.
• I'll polish what I have before creating a PR (targeting Florian's branch? v6?)

[florian-lefebvre]

I think having a dedicated page would make sense yeah!

In the Adapter API page I introduced a "Building adapter entrypoints" section in anticipation of the "Building a preview entrypoint" section. And "Passing build-time configuration" is probably for all entrypoints so we don't need to nest it under "Building a server entrypoint".

No this is specific to the server entrypoint, the preview entrypoint cannot use the same mechanism

I also noticed there are a lot of available imports from astro/app we don't document.

I don't know either! I have the feeling some of these are meant to be used internally so I would only document App for now

I'll polish what I have before creating a PR (targeting Florian's branch? v6?)

Feel free to do it in this PR 👍

[sarah11918]

I guess a separate page would help future proof us for Algolia complaining the page is too long again 😅

My only concern is that astro/app is only going to be used by integration/adapter authors, right? But for most people coming from e.g. React or a React-based framework, they are used to thinking of App as their "project entry point". So, people might think this is part of building an Astro site/"app" itself and we'd want to make it super clear that this is for integration authors.

Note that I don't think making it more visible it is necessarily a bad thing, though! I see Florian suggesting in support all the time, "Oh, you need this functionality, you can write an integration for that! So, integrations are a natural extension of Astro behaviour, and maybe one we should be encouraging! Again, my only concern would be people inadvertantly getting in there, getting confused etc. when they're just making a basic site, so clarity is key.

But if that entire section of the Adapter API page is/would be simply a string of API references anyway, then moving it off to the modules section does make sense, I think!

[ArmandPhilippot]

Yeah, that was also my concern re: "think about a better name than "App API Reference" for the astro/app page".

But having one module documented here while all the others (including astro:static-paths which is for integrations/adapter too) have their own page is a bit odd. And, yes, I think this helps both being future-proof with Algolia and dealing with nested sections.

No this is specific to the server entrypoint, the preview entrypoint cannot use the same mechanism

Oh, thanks for pushing back! I assumed they worked in the same way.

Alright, I'll polish what I have and I'll push the changes here. If we don't like them, we can always revert the commit!

[ArmandPhilippot]

So tiny! 😅

Well, not so tiny anymore. 😆

There is probably some wording to polish but I think all the changes are here, including broken links fixes because of the reorganization!

To summarize what I've done:

• I moved all the astro/app docs to a new page in "Runtime APIs"
• I took the third path, ie. "we document App under createApp()" because I don't see how we can get manifest (to satisfy the App constructor) and we document what is public/what is useful for adapters authors, not what is helpful for core maintainers...
• And because of the point above, I removed all the static methods (anyway, two of them are already removed in core)... maybe we want to add something in the upgrade guide just in case?
• While I was there (ie. in the new astro/app page):
  • I updated the way we were introducing the module to call out this is for adapters authors
  • I added more links
  • I updated the RenderOptions.addCookieHeader description to remove "When this is false..." because it seems this edge case is only relevant for core maintainers now
  • I updated the createApp() description
  • I updated the last two code snippets to include createApp() (my thinking is, if people are interest in Node specifically, this might be useful to have a picture of how things unfold)
• I updated the "Building a server entrypoint" description to add links to the new page

[florian-lefebvre]

Not sure if types should come first, or if the "runtime" should instead

[ArmandPhilippot]

Yeah, I get what you mean. I hesitated. I left it there because I thought it might be confusing for the user to go into astro/app and not see any "direct" import option from that module (if that makes sense 😅 ).
But looking back, I think you you're right and we can move them at the bottom!

[florian-lefebvre]

Looks pretty good!

[sarah11918]

I just want to note, for the record, this was Florian's original PR:

Team Docs, FTW! 💪

[yanthomasdev]

A couple nits from me

[sarah11918]

This looks fantastic @ArmandPhilippot !! (Not so tiny, indeed!) Just some polishing comments from me!

[sarah11918]

Really nice work here! Love this intro to the module!

[florian-lefebvre]

Very nice! I can't approve my own PR 😆 but huge LGTM!!

[ArmandPhilippot]

I love seeing how moving content helps to refine what we had! Great teamwork!

If everyone is happy, I think this one is ready to be merged! 🎉

[sarah11918]

The world's best docs team! 🙌

[sarah11918]

Just noting that we have the one ru page being updated here, so we'll need a Lunaria statement for that!

[ArmandPhilippot]

This is for v6 though, so we'll need to deal with Lunaria on the v6 PR not here, right?

[sarah11918]

Oh, forgot! Yes, should be fine! Thank you!

[florian-lefebvre]

Alright then can this PR be merged? The associated code PR was released last week already so we're not blocked by that

[ArmandPhilippot]

Yeah, Sarah approved it! I know I hijacked your PR but this is still your PR so I was waiting for you to merge it just in case you had second thoughts. 😄