feat: add streaming arg to createApp()

[florian-lefebvre]

Changes

• Improves the new adapter API by allowing to pass streaming to createApp(), matching App

Testing

N/A

Docs

• Changeset
• feat: add streaming arg to createApp() docs#13273

[changeset-bot]

🦋 Changeset detected

Latest commit: ba08958

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

[matthewp]

If no adapters are going to use this, why add it?

[codspeed-hq]

Merging this PR will not alter performance

✅ 9 untouched benchmarks

---

_{Comparing feat/create-app-streaming-argument (ba08958) with main (9760404)}

[ematipico]

If no adapters are going to use this, why add it?

IIRC, the node adapter uses it

[florian-lefebvre]

If no adapters are going to use this, why add it?

There's no valid reason IMO to make createApp() less powerful than App. We don't use the streaming arg in core but that doesn't mean it's not used by 3rd party adapters. Like really it's not about adding a feature, it's about making sure it enables the same stuff as App.

IIRC, the node adapter uses it

The node adapter uses NodeApp so it's different, I have another PR in the works for it

[matthewp]

@florian-lefebvre For context, we added streaming false because there was an adapter that did not support streaming (I think it was Cloudflare at the time), but I don't think that's the case any more. So I guess I'm questioning the value of the feature in App as well.

[florian-lefebvre]

Ah gotcha, that's different. I think we probably don't want to remove streaming from App now but we could deprecate it if we wanted. Honestly I think it doesn't hurt if it's here either, no strong opinion. So to recap:

• We want to remove the streaming arg from App, so we deprecate it in v6 and do not add the flag to createApp
• We want to keep supporting it and we update createApp

How can we decide properly?

[matthewp]

I don't want to block and don't have time to create a proposal for removal, so since it already exists I won't oppose.

[florian-lefebvre]

With NodeApp getting deprecated soon (#15535), having the streaming arg on App and createApp() makes even more sense

[ascorbic]

Approving subject to a clarification in the changeset. I think this is a good option that should possibly be exposed by adapters

[florian-lefebvre]

Yeah I agree being able to set streaming for all adapters would be good, but IIRC not everyone was in favor hence why the node adapter is the only one to have this feature. Personally happy to revisit but that's not blocked by v6 and may need an RFC?

[sarah11918]

OK, here are my thoughts on the changeset, @florian-lefebvre !

[sarah11918]

Thanks for your persistence! I think this looks good! (I'd remove the extra line, but no complaints from me!)