BuilderIO/builder

Impossible to implement ISR on Next.js App Router

neg4n opened this issue · 1 comments

Describe the bug
@builder.io/sdk@2.2.1 package sets the next: { revalidate: 1 } for all fetch requests. This disables page-specific revalidate option (export const revalidate = 1800) in Next.js App Router since all the options specified in fetch overrides the page-level revalidation. This makes incremental static regeneration implementation impossible on Next.js App Router.

To Reproduce
Steps to reproduce the behavior:

  1. Create new Next.js App Router project
  2. Integrate Builder SDK and create some page (e.g. app/[...page]/page.tsx) for catching all routes
  3. Add getStaticPaths for generating routes and do some fetching using builder.get in the Server Component. (you can wrap this in cache() from react in order to dedupe requests)
  4. Add
    export const revalidate = 1800
    export const fetchCache = "force-cache"
    export const dynamic = "force-static"
    at the end of the page.tsx. The revalidation will happen every 1 second not 1800!

Expected behavior
The best approach would be to remove the default options for next: {} extended fetch within the Builder SDK. While this can be bypassed by using unstable_cache, it also indicates that all page content is dynamically served, except for the items wrapped in unstable_cache. When using page-level revalidation with ISR, the optimization is significantly higher.

Additional context
I've managed to solve the problem using patch-package. Here's the diff

diff --git a/node_modules/@builder.io/sdk/dist/index.browser.js b/node_modules/@builder.io/sdk/dist/index.browser.js
index f8b5738..c84fc74 100644
--- a/node_modules/@builder.io/sdk/dist/index.browser.js
+++ b/node_modules/@builder.io/sdk/dist/index.browser.js
@@ -2342,7 +2342,7 @@
         // even though we only use `fetch()` now, we prefer to keep the old behavior and use the `fetch` that comes from
         // the core SDK for consistency
         Builder.prototype.requestUrl = function (url, options) {
-            return getFetch()(url, __assign({ next: __assign({ revalidate: 1 }, options === null || options === void 0 ? void 0 : options.next) }, options)).then(function (res) { return res.json(); });
+            return getFetch()(url, options).then(function (res) { return res.json(); });
         };
         Object.defineProperty(Builder.prototype, "host", {
             get: function () {
@@ -2516,7 +2516,7 @@
             }
             var queryStr = QueryString.stringifyDeep(queryParams);
             var format = queryParams.format;
-            var requestOptions = { headers: {}, next: { revalidate: 1 } };
+            var requestOptions = { headers: {} };
             if (this.authToken) {
                 requestOptions.headers = __assign(__assign({}, requestOptions.headers), { Authorization: "Bearer ".concat(this.authToken) });
             }
diff --git a/node_modules/@builder.io/sdk/dist/index.cjs.js b/node_modules/@builder.io/sdk/dist/index.cjs.js
index 8ca3cf4..96a325c 100644
--- a/node_modules/@builder.io/sdk/dist/index.cjs.js
+++ b/node_modules/@builder.io/sdk/dist/index.cjs.js
@@ -2248,7 +2248,7 @@ var Builder = /** @class */ (function () {
     // even though we only use `fetch()` now, we prefer to keep the old behavior and use the `fetch` that comes from
     // the core SDK for consistency
     Builder.prototype.requestUrl = function (url, options) {
-        return getFetch()(url, tslib.__assign({ next: tslib.__assign({ revalidate: 1 }, options === null || options === void 0 ? void 0 : options.next) }, options)).then(function (res) { return res.json(); });
+        return getFetch()(url, options).then(function (res) { return res.json(); });
     };
     Object.defineProperty(Builder.prototype, "host", {
         get: function () {
@@ -2422,7 +2422,7 @@ var Builder = /** @class */ (function () {
         }
         var queryStr = QueryString.stringifyDeep(queryParams);
         var format = queryParams.format;
-        var requestOptions = { headers: {}, next: { revalidate: 1 } };
+        var requestOptions = { headers: {} };
         if (this.authToken) {
             requestOptions.headers = tslib.__assign(tslib.__assign({}, requestOptions.headers), { Authorization: "Bearer ".concat(this.authToken) });
         }
diff --git a/node_modules/@builder.io/sdk/dist/index.esm.js b/node_modules/@builder.io/sdk/dist/index.esm.js
index 5defb3e..438a773 100644
--- a/node_modules/@builder.io/sdk/dist/index.esm.js
+++ b/node_modules/@builder.io/sdk/dist/index.esm.js
@@ -2240,7 +2240,7 @@ var Builder = /** @class */ (function () {
     // even though we only use `fetch()` now, we prefer to keep the old behavior and use the `fetch` that comes from
     // the core SDK for consistency
     Builder.prototype.requestUrl = function (url, options) {
-        return getFetch()(url, __assign({ next: __assign({ revalidate: 1 }, options === null || options === void 0 ? void 0 : options.next) }, options)).then(function (res) { return res.json(); });
+        return getFetch()(url, options).then(function (res) { return res.json(); });
     };
     Object.defineProperty(Builder.prototype, "host", {
         get: function () {
@@ -2414,7 +2414,7 @@ var Builder = /** @class */ (function () {
         }
         var queryStr = QueryString.stringifyDeep(queryParams);
         var format = queryParams.format;
-        var requestOptions = { headers: {}, next: { revalidate: 1 } };
+        var requestOptions = { headers: {} };
         if (this.authToken) {
             requestOptions.headers = __assign(__assign({}, requestOptions.headers), { Authorization: "Bearer ".concat(this.authToken) });
         }
diff --git a/node_modules/@builder.io/sdk/dist/index.umd.js b/node_modules/@builder.io/sdk/dist/index.umd.js
index f37ba5b..e4d5bc9 100644
--- a/node_modules/@builder.io/sdk/dist/index.umd.js
+++ b/node_modules/@builder.io/sdk/dist/index.umd.js
@@ -2342,7 +2342,7 @@
         // even though we only use `fetch()` now, we prefer to keep the old behavior and use the `fetch` that comes from
         // the core SDK for consistency
         Builder.prototype.requestUrl = function (url, options) {
-            return getFetch()(url, __assign({ next: __assign({ revalidate: 1 }, options === null || options === void 0 ? void 0 : options.next) }, options)).then(function (res) { return res.json(); });
+            return getFetch()(url, options).then(function (res) { return res.json(); });
         };
         Object.defineProperty(Builder.prototype, "host", {
             get: function () {
@@ -2516,7 +2516,7 @@
             }
             var queryStr = QueryString.stringifyDeep(queryParams);
             var format = queryParams.format;
-            var requestOptions = { headers: {}, next: { revalidate: 1 } };
+            var requestOptions = { headers: {} };
             if (this.authToken) {
                 requestOptions.headers = __assign(__assign({}, requestOptions.headers), { Authorization: "Bearer ".concat(this.authToken) });
             }
diff --git a/node_modules/@builder.io/sdk/dist/src/builder.class.js b/node_modules/@builder.io/sdk/dist/src/builder.class.js
index 06459a6..f81eb62 100644
--- a/node_modules/@builder.io/sdk/dist/src/builder.class.js
+++ b/node_modules/@builder.io/sdk/dist/src/builder.class.js
@@ -1310,7 +1310,7 @@ var Builder = /** @class */ (function () {
     // even though we only use `fetch()` now, we prefer to keep the old behavior and use the `fetch` that comes from
     // the core SDK for consistency
     Builder.prototype.requestUrl = function (url, options) {
-        return (0, fetch_function_1.getFetch)()(url, __assign({ next: __assign({ revalidate: 1 }, options === null || options === void 0 ? void 0 : options.next) }, options)).then(function (res) { return res.json(); });
+        return (0, fetch_function_1.getFetch)()(url, options).then(function (res) { return res.json(); });
     };
     Object.defineProperty(Builder.prototype, "host", {
         get: function () {
@@ -1484,7 +1484,7 @@ var Builder = /** @class */ (function () {
         }
         var queryStr = query_string_class_1.QueryString.stringifyDeep(queryParams);
         var format = queryParams.format;
-        var requestOptions = { headers: {}, next: { revalidate: 1 } };
+        var requestOptions = { headers: {} };
         if (this.authToken) {
             requestOptions.headers = __assign(__assign({}, requestOptions.headers), { Authorization: "Bearer ".concat(this.authToken) });
         }

thanks for the thorough report @neg4n - PR with the fix sent and should get merged/published in the next week or so when Sami is back