Skip to content

Commit e87d8bb

Browse files
conico974Nicolas Dorseuil
andauthored
Fix fetch inside use cache in ISR (#666)
* fix use cache with fetch and ISR * add e2e test * lint fix * changeset * add comment --------- Co-authored-by: Nicolas Dorseuil <nicolas@gitbook.io>
1 parent 7b06a42 commit e87d8bb

File tree

6 files changed

+272
-0
lines changed

6 files changed

+272
-0
lines changed

.changeset/major-mails-shave.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@opennextjs/cloudflare": patch
3+
---
4+
5+
fix fetch inside `use cache` in ISR

examples/e2e/experimental/e2e/use-cache.test.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,45 @@ test.describe("Composable Cache", () => {
8383
const fullyCachedText = await fullyCachedElt.textContent();
8484
expect(fullyCachedText).toEqual(initialFullyCachedText);
8585
});
86+
87+
test("cached fetch should work in ISR", async ({ page }) => {
88+
await page.goto("/use-cache/fetch");
89+
90+
let dateElt = page.getByTestId("date");
91+
await expect(dateElt).toBeVisible();
92+
93+
let initialDate = await dateElt.textContent();
94+
95+
let isrElt = page.getByTestId("isr");
96+
await expect(isrElt).toBeVisible();
97+
let initialIsrText = await isrElt.textContent();
98+
99+
// We have to force reload until ISR has triggered at least once, otherwise the test will be flakey
100+
101+
let isrText = initialIsrText;
102+
103+
while (isrText === initialIsrText) {
104+
await page.reload();
105+
isrElt = page.getByTestId("isr");
106+
dateElt = page.getByTestId("date");
107+
await expect(isrElt).toBeVisible();
108+
isrText = await isrElt.textContent();
109+
await expect(dateElt).toBeVisible();
110+
initialDate = await dateElt.textContent();
111+
await page.waitForTimeout(1000);
112+
}
113+
initialIsrText = isrText;
114+
115+
do {
116+
await page.reload();
117+
dateElt = page.getByTestId("date");
118+
isrElt = page.getByTestId("isr");
119+
await expect(dateElt).toBeVisible();
120+
await expect(isrElt).toBeVisible();
121+
isrText = await isrElt.textContent();
122+
await page.waitForTimeout(1000);
123+
} while (isrText === initialIsrText);
124+
const fullyCachedText = await dateElt.textContent();
125+
expect(fullyCachedText).toEqual(initialDate);
126+
});
86127
});
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import { ISRComponent } from "@/components/cached";
2+
import { Suspense } from "react";
3+
4+
async function getFromFetch() {
5+
"use cache";
6+
// This is a simple fetch to ensure that the cache is working with IO inside
7+
const res = await fetch("https://opennext.js.org");
8+
return res.headers.get("Date");
9+
}
10+
11+
export default async function Page() {
12+
const date = await getFromFetch();
13+
return (
14+
<div>
15+
<h1>Cache</h1>
16+
<p data-testid="date">{date}</p>
17+
<Suspense fallback={<p>Loading...</p>}>
18+
<ISRComponent />
19+
</Suspense>
20+
</div>
21+
);
22+
}

packages/cloudflare/src/cli/build/open-next/createServerBundle.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import type { Plugin } from "esbuild";
3636

3737
import { getOpenNextConfig } from "../../../api/config.js";
3838
import { patchResRevalidate } from "../patches/plugins/res-revalidate.js";
39+
import { patchUseCacheIO } from "../patches/plugins/use-cache.js";
3940
import { normalizePath } from "../utils/index.js";
4041
import { copyWorkerdPackages } from "../utils/workerd.js";
4142

@@ -216,6 +217,7 @@ async function generateBundle(
216217
patchBackgroundRevalidation,
217218
// Cloudflare specific patches
218219
patchResRevalidate,
220+
patchUseCacheIO,
219221
...additionalCodePatches,
220222
]);
221223

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
import { patchCode } from "@opennextjs/aws/build/patch/astCodePatcher.js";
2+
import { expect, test } from "vitest";
3+
4+
import { rule } from "./use-cache.js";
5+
6+
const codeToPatch = `"use strict";
7+
Object.defineProperty(exports, "__esModule", {
8+
value: true
9+
});
10+
0 && (module.exports = {
11+
bindSnapshot: null,
12+
createAsyncLocalStorage: null,
13+
createSnapshot: null
14+
});
15+
function _export(target, all) {
16+
for(var name in all)Object.defineProperty(target, name, {
17+
enumerable: true,
18+
get: all[name]
19+
});
20+
}
21+
_export(exports, {
22+
bindSnapshot: function() {
23+
return bindSnapshot;
24+
},
25+
createAsyncLocalStorage: function() {
26+
return createAsyncLocalStorage;
27+
},
28+
createSnapshot: function() {
29+
return createSnapshot;
30+
}
31+
});
32+
const sharedAsyncLocalStorageNotAvailableError = Object.defineProperty(new Error('Invariant: AsyncLocalStorage accessed in runtime where it is not available'), "__NEXT_ERROR_CODE", {
33+
value: "E504",
34+
enumerable: false,
35+
configurable: true
36+
});
37+
class FakeAsyncLocalStorage {
38+
disable() {
39+
throw sharedAsyncLocalStorageNotAvailableError;
40+
}
41+
getStore() {
42+
// This fake implementation of AsyncLocalStorage always returns \`undefined\`.
43+
return undefined;
44+
}
45+
run() {
46+
throw sharedAsyncLocalStorageNotAvailableError;
47+
}
48+
exit() {
49+
throw sharedAsyncLocalStorageNotAvailableError;
50+
}
51+
enterWith() {
52+
throw sharedAsyncLocalStorageNotAvailableError;
53+
}
54+
static bind(fn) {
55+
return fn;
56+
}
57+
}
58+
const maybeGlobalAsyncLocalStorage = typeof globalThis !== 'undefined' && globalThis.AsyncLocalStorage;
59+
function createAsyncLocalStorage() {
60+
if (maybeGlobalAsyncLocalStorage) {
61+
return new maybeGlobalAsyncLocalStorage();
62+
}
63+
return new FakeAsyncLocalStorage();
64+
}
65+
function bindSnapshot(fn) {
66+
if (maybeGlobalAsyncLocalStorage) {
67+
return maybeGlobalAsyncLocalStorage.bind(fn);
68+
}
69+
return FakeAsyncLocalStorage.bind(fn);
70+
}
71+
function createSnapshot() {
72+
if (maybeGlobalAsyncLocalStorage) {
73+
return maybeGlobalAsyncLocalStorage.snapshot();
74+
}
75+
return function(fn, ...args) {
76+
return fn(...args);
77+
};
78+
}
79+
80+
//# sourceMappingURL=async-local-storage.js.map
81+
`;
82+
83+
test("patch the createSnapshot function", () => {
84+
const patchedCode = patchCode(codeToPatch, rule);
85+
expect(patchedCode).toMatchInlineSnapshot(`""use strict";
86+
Object.defineProperty(exports, "__esModule", {
87+
value: true
88+
});
89+
0 && (module.exports = {
90+
bindSnapshot: null,
91+
createAsyncLocalStorage: null,
92+
createSnapshot: null
93+
});
94+
function _export(target, all) {
95+
for(var name in all)Object.defineProperty(target, name, {
96+
enumerable: true,
97+
get: all[name]
98+
});
99+
}
100+
_export(exports, {
101+
bindSnapshot: function() {
102+
return bindSnapshot;
103+
},
104+
createAsyncLocalStorage: function() {
105+
return createAsyncLocalStorage;
106+
},
107+
createSnapshot: function() {
108+
return createSnapshot;
109+
}
110+
});
111+
const sharedAsyncLocalStorageNotAvailableError = Object.defineProperty(new Error('Invariant: AsyncLocalStorage accessed in runtime where it is not available'), "__NEXT_ERROR_CODE", {
112+
value: "E504",
113+
enumerable: false,
114+
configurable: true
115+
});
116+
class FakeAsyncLocalStorage {
117+
disable() {
118+
throw sharedAsyncLocalStorageNotAvailableError;
119+
}
120+
getStore() {
121+
// This fake implementation of AsyncLocalStorage always returns \`undefined\`.
122+
return undefined;
123+
}
124+
run() {
125+
throw sharedAsyncLocalStorageNotAvailableError;
126+
}
127+
exit() {
128+
throw sharedAsyncLocalStorageNotAvailableError;
129+
}
130+
enterWith() {
131+
throw sharedAsyncLocalStorageNotAvailableError;
132+
}
133+
static bind(fn) {
134+
return fn;
135+
}
136+
}
137+
const maybeGlobalAsyncLocalStorage = typeof globalThis !== 'undefined' && globalThis.AsyncLocalStorage;
138+
function createAsyncLocalStorage() {
139+
if (maybeGlobalAsyncLocalStorage) {
140+
return new maybeGlobalAsyncLocalStorage();
141+
}
142+
return new FakeAsyncLocalStorage();
143+
}
144+
function bindSnapshot(fn) {
145+
if (maybeGlobalAsyncLocalStorage) {
146+
return maybeGlobalAsyncLocalStorage.bind(fn);
147+
}
148+
return FakeAsyncLocalStorage.bind(fn);
149+
}
150+
function createSnapshot() {
151+
// Ignored snapshot
152+
return function(fn, ...args) {
153+
return fn(...args);
154+
};
155+
}
156+
157+
//# sourceMappingURL=async-local-storage.js.map
158+
"`);
159+
});
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* This patch will replace the createSnapshot function in the
3+
* server/app-render/async-local-storage.js file to an empty string.
4+
* This is necessary because the createSnapshot function is causing I/O issues for
5+
* ISR/SSG revalidation in Cloudflare Workers.
6+
* This is because by default it will use AsyncLocalStorage.snapshot() and it will
7+
* bind everything to the initial request context.
8+
* The downsides is that use cache function will have access to the full request
9+
* ALS context from next (i.e. cookies, headers ...)
10+
* TODO: Find a better fix for this issue.
11+
*/
12+
import { patchCode } from "@opennextjs/aws/build/patch/astCodePatcher.js";
13+
import type { CodePatcher } from "@opennextjs/aws/build/patch/codePatcher.js";
14+
import { getCrossPlatformPathRegex } from "@opennextjs/aws/utils/regex.js";
15+
16+
export const rule = `
17+
rule:
18+
kind: if_statement
19+
inside:
20+
kind: function_declaration
21+
stopBy: end
22+
has:
23+
kind: identifier
24+
pattern: createSnapshot
25+
fix:
26+
'// Ignored snapshot'
27+
`;
28+
29+
export const patchUseCacheIO: CodePatcher = {
30+
name: "patch-use-cache",
31+
patches: [
32+
{
33+
versions: ">=15.3.1",
34+
field: {
35+
pathFilter: getCrossPlatformPathRegex(String.raw`server/app-render/async-local-storage\.js$`, {
36+
escape: false,
37+
}),
38+
contentFilter: /createSnapshot/,
39+
patchCode: async ({ code }) => patchCode(code, rule),
40+
},
41+
},
42+
],
43+
};

0 commit comments

Comments
 (0)