feat: add a cloudflare-streaming wrapper

[vicb]

@conico974 Do you think "cloudflare-streaming" is a reasonable name or "cloudflare-server" would be better (or any other idea)?

The changes are somehow tested - might require minor tweaks later.

I have spread the changes across different commits per unit of work to ease the review.

Thanks!

[changeset-bot]

🦋 Changeset detected

Latest commit: 1e9aa86

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

This PR includes changesets to release 3 packages

Name	Type
@opennextjs/aws	Minor
app-pages-router	Patch
app-router	Patch

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

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	22.02%	1659 / 7531
🔵	Statements	22.02%	1659 / 7531
🔵	Functions	56.25%	99 / 176
🔵	Branches	70.28%	414 / 589

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
packages/open-next/src/build/createServerBundle.ts	0%	0%	0%	0%	1-315
packages/open-next/src/build/validateConfig.ts	0%	0%	0%	0%	1-115
packages/open-next/src/overrides/converters/aws-apigw-v2.ts	91.37%	83.33%	100%	91.37%	45-46, 50-54, 113-115
packages/open-next/src/overrides/converters/edge.ts	0%	0%	0%	0%	1-114
packages/open-next/src/overrides/wrappers/cloudflare-streaming.ts	0%	0%	0%	0%	1-77
packages/open-next/src/overrides/wrappers/cloudflare.ts	0%	100%	100%	0%	6-68
packages/open-next/src/types/open-next.ts	0%	0%	0%	0%

Generated in workflow #775 for commit 1e9aa86 by the Vitest Coverage Report Action

[pkg-pr-new]

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/aws@642

commit: 1e9aa86

[conico974]

I think it would make more sense to use cloudflare-server instead of cloudflare-streaming.
The other one support streaming as well, it's just that it only works with next edge runtime.

Even better we could rename the other one cloudflare-edge and this one cloudflare-node which would allow people to understand more easily which one to use depending on the runtime they use

[conico974]

Wouldn't this be necessary to import process here ?
Something like import * as process from 'node:process';

[vicb]

process is a global in node compat mode so I think this should be dropped entirely. This code would only make sense when not in node compat mode.

[conico974]

I'm not entirely sure what the value should be here.
My take is that it should be false as this is used for the edge runtime of next which is not the use case for this wrapper.
If that's the way we decide to pursue, we should add a check in the validation of the config file to throw an error for people using edgeRuntime wrapper without runtime: "edge" in their config
It won't have any effect anyway if used on a "node" route

[vicb]

I added this value without paying too much attention but I agree it should be false.

[vicb]

Even better we could rename the other one cloudflare-edge and this one cloudflare-node which would allow people to understand more easily which one to use depending on the runtime they use

Great idea. I'll do that (and alias the former "cloudflare" to "cloudflare-edge" for BC!

[vicb]

@conico974 do you think we could merge this PR as is and I would address the comments in a follow up early next week. The rationale is that I have a pending opennextjs/opennextjs-cloudflare#140 using this and I think if I push here it might update the pre-release and break that PR (not sure if the version changes or not).

[conico974]

Yeah we can merge it's fine. Thanks