-
Notifications
You must be signed in to change notification settings - Fork 72
Fix for next 15.2 and renderHTML should not be called in minimal mode #441
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
6d196c5
fix issue
conico974 ecd4cd5
bump next in e2e to 15.2
conico974 4b7cabe
added a small test
conico974 1943441
linting
conico974 40fec7d
fix for experimental react
conico974 236c84d
define __NEXT_EXPERIMENTAL_REACT to reduce bundle size
conico974 6394601
review fix
conico974 b89abd9
changeset
conico974 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
packages/cloudflare/src/cli/build/patches/plugins/next-minimal.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
import { describe, expect, test } from "vitest"; | ||
|
||
import { patchCode } from "../ast/util"; | ||
import { abortControllerRule } from "./next-minimal"; | ||
|
||
const appPageRuntimeProdJs = `let p = new AbortController; | ||
vicb marked this conversation as resolved.
Show resolved
Hide resolved
|
||
async function h(e3, t3) { | ||
let { flightRouterState: r3, nextUrl: a2, prefetchKind: i2 } = t3, u2 = { [n2.hY]: "1", [n2.B]: encodeURIComponent(JSON.stringify(r3)) }; | ||
i2 === o.ob.AUTO && (u2[n2._V] = "1"), a2 && (u2[n2.kO] = a2); | ||
try { | ||
var c2; | ||
let t4 = i2 ? i2 === o.ob.TEMPORARY ? "high" : "low" : "auto"; | ||
"export" === process.env.__NEXT_CONFIG_OUTPUT && ((e3 = new URL(e3)).pathname.endsWith("/") ? e3.pathname += "index.txt" : e3.pathname += ".txt"); | ||
let r4 = await m(e3, u2, t4, p.signal), a3 = d(r4.url), h2 = r4.redirected ? a3 : void 0, g = r4.headers.get("content-type") || "", v = !!(null == (c2 = r4.headers.get("vary")) ? void 0 : c2.includes(n2.kO)), b = !!r4.headers.get(n2.jc), S = r4.headers.get(n2.UK), _ = null !== S ? parseInt(S, 10) : -1, w = g.startsWith(n2.al); | ||
if ("export" !== process.env.__NEXT_CONFIG_OUTPUT || w || (w = g.startsWith("text/plain")), !w || !r4.ok || !r4.body) | ||
return e3.hash && (a3.hash = e3.hash), f(a3.toString()); | ||
let k = b ? function(e4) { | ||
let t5 = e4.getReader(); | ||
return new ReadableStream({ async pull(e5) { | ||
for (; ; ) { | ||
let { done: r5, value: n3 } = await t5.read(); | ||
if (!r5) { | ||
e5.enqueue(n3); | ||
continue; | ||
} | ||
return; | ||
} | ||
} }); | ||
}(r4.body) : r4.body, E = await y(k); | ||
if ((0, l.X)() !== E.b) | ||
return f(r4.url); | ||
return { flightData: (0, s.aj)(E.f), canonicalUrl: h2, couldBeIntercepted: v, prerendered: E.S, postponed: b, staleTime: _ }; | ||
} catch (t4) { | ||
return p.signal.aborted || console.error("Failed to fetch RSC payload for " + e3 + ". Falling back to browser navigation.", t4), { flightData: e3.toString(), canonicalUrl: void 0, couldBeIntercepted: false, prerendered: false, postponed: false, staleTime: -1 }; | ||
} | ||
} | ||
`; | ||
|
||
describe("Abort controller", () => { | ||
test("minimal", () => { | ||
expect(patchCode(appPageRuntimeProdJs, abortControllerRule)).toBe( | ||
`let p = {signal:{aborted: false}}; | ||
async function h(e3, t3) { | ||
let { flightRouterState: r3, nextUrl: a2, prefetchKind: i2 } = t3, u2 = { [n2.hY]: "1", [n2.B]: encodeURIComponent(JSON.stringify(r3)) }; | ||
i2 === o.ob.AUTO && (u2[n2._V] = "1"), a2 && (u2[n2.kO] = a2); | ||
try { | ||
var c2; | ||
let t4 = i2 ? i2 === o.ob.TEMPORARY ? "high" : "low" : "auto"; | ||
"export" === process.env.__NEXT_CONFIG_OUTPUT && ((e3 = new URL(e3)).pathname.endsWith("/") ? e3.pathname += "index.txt" : e3.pathname += ".txt"); | ||
let r4 = await m(e3, u2, t4, p.signal), a3 = d(r4.url), h2 = r4.redirected ? a3 : void 0, g = r4.headers.get("content-type") || "", v = !!(null == (c2 = r4.headers.get("vary")) ? void 0 : c2.includes(n2.kO)), b = !!r4.headers.get(n2.jc), S = r4.headers.get(n2.UK), _ = null !== S ? parseInt(S, 10) : -1, w = g.startsWith(n2.al); | ||
if ("export" !== process.env.__NEXT_CONFIG_OUTPUT || w || (w = g.startsWith("text/plain")), !w || !r4.ok || !r4.body) | ||
return e3.hash && (a3.hash = e3.hash), f(a3.toString()); | ||
let k = b ? function(e4) { | ||
let t5 = e4.getReader(); | ||
return new ReadableStream({ async pull(e5) { | ||
for (; ; ) { | ||
let { done: r5, value: n3 } = await t5.read(); | ||
if (!r5) { | ||
e5.enqueue(n3); | ||
continue; | ||
} | ||
return; | ||
} | ||
} }); | ||
}(r4.body) : r4.body, E = await y(k); | ||
if ((0, l.X)() !== E.b) | ||
return f(r4.url); | ||
return { flightData: (0, s.aj)(E.f), canonicalUrl: h2, couldBeIntercepted: v, prerendered: E.S, postponed: b, staleTime: _ }; | ||
} catch (t4) { | ||
return p.signal.aborted || console.error("Failed to fetch RSC payload for " + e3 + ". Falling back to browser navigation.", t4), { flightData: e3.toString(), canonicalUrl: void 0, couldBeIntercepted: false, prerendered: false, postponed: false, staleTime: -1 }; | ||
} | ||
} | ||
` | ||
); | ||
}); | ||
}); |
92 changes: 92 additions & 0 deletions
92
packages/cloudflare/src/cli/build/patches/plugins/next-minimal.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import { patchCode } from "../ast/util.js"; | ||
import { ContentUpdater } from "./content-updater.js"; | ||
|
||
// We try to be as specific as possible to avoid patching the wrong thing here | ||
// It seems that there is a bug in the worker runtime. When the AbortController is created outside of the request context it throws an error (not sure if it's expected or not) except in this case. | ||
vicb marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
conico974 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
// It fails while requiring the `app-page.runtime.prod.js` file, but instead of throwing an error, it just return an empty object for the `require('app-page.runtime.prod.js')` call which makes every request to an app router page fail. | ||
// If it's a bug in workerd and it's not expected to throw an error, we can remove this patch. | ||
export const abortControllerRule = ` | ||
rule: | ||
all: | ||
- kind: lexical_declaration | ||
pattern: let $VAR = new AbortController | ||
- precedes: | ||
kind: function_declaration | ||
stopBy: end | ||
has: | ||
kind: statement_block | ||
has: | ||
kind: try_statement | ||
has: | ||
kind: catch_clause | ||
has: | ||
kind: statement_block | ||
conico974 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
has: | ||
kind: return_statement | ||
all: | ||
- has: | ||
stopBy: end | ||
kind: member_expression | ||
pattern: $VAR.signal.aborted | ||
- has: | ||
stopBy: end | ||
kind: call_expression | ||
regex: console.error\\("Failed to fetch RSC payload for | ||
fix: | ||
'let $VAR = {signal:{aborted: false}};' | ||
`; | ||
|
||
// This rule is used instead of defining `process.env.NEXT_MINIMAL` in the `esbuild config. | ||
// Do we want to entirely replace these functions to reduce the bundle size? | ||
// In next `renderHTML` is used as a fallback in case of errors, but in minimal mode it just throws the error and the responsability of handling it is on the infra. | ||
export const nextMinimalRule = ` | ||
rule: | ||
kind: member_expression | ||
pattern: process.env.NEXT_MINIMAL | ||
any: | ||
- inside: | ||
kind: parenthesized_expression | ||
stopBy: end | ||
inside: | ||
kind: if_statement | ||
any: | ||
- inside: | ||
kind: statement_block | ||
inside: | ||
kind: method_definition | ||
any: | ||
- has: {kind: property_identifier, field: name, regex: runEdgeFunction} | ||
- has: {kind: property_identifier, field: name, regex: runMiddleware} | ||
- has: {kind: property_identifier, field: name, regex: imageOptimizer} | ||
- has: | ||
kind: statement_block | ||
has: | ||
kind: expression_statement | ||
pattern: res.statusCode = 400; | ||
fix: | ||
'true' | ||
`; | ||
|
||
export function patchNextMinimal(updater: ContentUpdater) { | ||
updater.updateContent( | ||
"patch-abortController-next15.2", | ||
{ filter: /app-page(-experimental)?\.runtime\.prod\.(js)$/, contentFilter: /new AbortController/ }, | ||
conico974 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
async ({ contents }) => { | ||
return patchCode(contents, abortControllerRule); | ||
} | ||
); | ||
|
||
updater.updateContent( | ||
"patch-next-minimal", | ||
{ filter: /next-server\.(js)$/, contentFilter: /.*/ }, | ||
async ({ contents }) => { | ||
return patchCode(contents, nextMinimalRule); | ||
} | ||
); | ||
|
||
return { | ||
name: "patch-abortController", | ||
setup() {}, | ||
}; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
19 changes: 19 additions & 0 deletions
19
packages/cloudflare/src/cli/build/utils/needs-experimental-react.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import type { NextConfig } from "@opennextjs/aws/types/next-types"; | ||
|
||
// Not sure if this should be upstreamed to aws | ||
// Adding more stuff there make typing incorrect actually, these properties are never undefined as long as it is the right version of next | ||
// Ideally we'd have different `NextConfig` types for different versions of next | ||
interface ExtendedNextConfig extends NextConfig { | ||
experimental: { | ||
ppr?: boolean; | ||
taint?: boolean; | ||
viewTransition?: boolean; | ||
serverActions?: boolean; | ||
}; | ||
} | ||
|
||
// Copied from https://github.com/vercel/next.js/blob/4518bc91641a0fd938664b781e12ae7c145f3396/packages/next/src/lib/needs-experimental-react.ts#L3-L6 | ||
export function needsExperimentalReact(nextConfig: ExtendedNextConfig) { | ||
const { ppr, taint, viewTransition } = nextConfig.experimental || {}; | ||
return Boolean(ppr || taint || viewTransition); | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.