Skip to content
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

Make cacheLife profiles configurable #71232

Merged
merged 8 commits into from
Oct 14, 2024

Conversation

sebmarkbage
Copy link
Contributor

@sebmarkbage sebmarkbage commented Oct 13, 2024

This should really be a global config but we don't have an easy way to get the config from a module. Might need a loader. So this just plumbs it through a bunch of contexts.

I'm also generate the enum type based on the config.

@sebmarkbage sebmarkbage requested a review from ijjk October 13, 2024 22:18
@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Oct 13, 2024
Copy link

vercel bot commented Oct 13, 2024

Notifying the following users due to files changed in this PR based on this repo's notify modifiers:

@timneutkens, @ijjk, @shuding, @huozhi:

packages/next/src/server/config.ts

@ijjk
Copy link
Member

ijjk commented Oct 13, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Oct 13, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
buildDuration 19s 16.1s N/A
buildDurationCached 15.3s 13s N/A
nodeModulesSize 371 MB 371 MB ⚠️ +71.5 kB
nextStartRea..uration (ms) 413ms 415ms N/A
Client Bundles (main, webpack)
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
307bebc6-HASH.js gzip 52.6 kB 52.6 kB N/A
3620-HASH.js gzip 43.7 kB 43.7 kB
801.HASH.js gzip 168 B 169 B N/A
8962-HASH.js gzip 5.26 kB 5.26 kB N/A
framework-HASH.js gzip 57.4 kB 57.4 kB N/A
main-app-HASH.js gzip 231 B 231 B
main-HASH.js gzip 32.8 kB 32.8 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 44 kB 44 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
_app-HASH.js gzip 192 B 192 B
_error-HASH.js gzip 193 B 192 B N/A
amp-HASH.js gzip 509 B 509 B
css-HASH.js gzip 341 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 266 B 266 B
head-HASH.js gzip 365 B 364 B N/A
hooks-HASH.js gzip 392 B 391 B N/A
image-HASH.js gzip 4.41 kB 4.41 kB N/A
index-HASH.js gzip 267 B 268 B N/A
link-HASH.js gzip 2.78 kB 2.78 kB N/A
routerDirect..HASH.js gzip 327 B 327 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 324 B 322 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.64 kB 3.64 kB
Client Build Manifests
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
_buildManifest.js gzip 748 B 750 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
index.html gzip 521 B 523 B N/A
link.html gzip 536 B 537 B N/A
withRouter.html gzip 518 B 519 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
edge-ssr.js gzip 130 kB 130 kB N/A
page.js gzip 188 kB 188 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
middleware-b..fest.js gzip 671 B 671 B
middleware-r..fest.js gzip 154 B 156 B N/A
middleware.js gzip 30.7 kB 30.8 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 1.51 kB 1.51 kB
Next Runtimes
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
973-experime...dev.js gzip 322 B 322 B
973.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 314 kB 314 kB N/A
app-page-exp..prod.js gzip 120 kB 120 kB N/A
app-page-tur..prod.js gzip 134 kB 134 kB N/A
app-page-tur..prod.js gzip 129 kB 129 kB N/A
app-page.run...dev.js gzip 304 kB 304 kB N/A
app-page.run..prod.js gzip 116 kB 116 kB N/A
app-route-ex...dev.js gzip 35 kB 35.1 kB N/A
app-route-ex..prod.js gzip 23.7 kB 23.7 kB N/A
app-route-tu..prod.js gzip 23.7 kB 23.7 kB N/A
app-route-tu..prod.js gzip 23.5 kB 23.5 kB N/A
app-route.ru...dev.js gzip 36.7 kB 36.7 kB N/A
app-route.ru..prod.js gzip 23.5 kB 23.5 kB N/A
pages-api-tu..prod.js gzip 9.61 kB 9.61 kB
pages-api.ru...dev.js gzip 11.4 kB 11.4 kB
pages-api.ru..prod.js gzip 9.61 kB 9.61 kB
pages-turbo...prod.js gzip 20.9 kB 20.9 kB
pages.runtim...dev.js gzip 26.5 kB 26.5 kB
pages.runtim..prod.js gzip 20.9 kB 20.9 kB
server.runti..prod.js gzip 60.1 kB 60.2 kB N/A
Overall change 99.7 kB 99.7 kB
build cache Overall increase ⚠️
vercel/next.js canary sebmarkbage/next.js configurablecachelife Change
0.pack gzip 1.84 MB 1.84 MB ⚠️ +2.25 kB
index.pack gzip 142 kB 142 kB ⚠️ +549 B
Overall change 1.98 MB 1.99 MB ⚠️ +2.8 kB
Diff details
Diff for page.js

Diff too large to display

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for image-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [8358],
   {
-    /***/ 6441: /***/ (
+    /***/ 7486: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(2563);
+          return __webpack_require__(5412);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 6730: /***/ (module, exports, __webpack_require__) => {
+    /***/ 9569: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,17 +40,17 @@
         __webpack_require__(38)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(8624)
+        __webpack_require__(7269)
       );
-      const _getimgprops = __webpack_require__(2990);
-      const _imageconfig = __webpack_require__(876);
-      const _imageconfigcontextsharedruntime = __webpack_require__(2767);
-      const _warnonce = __webpack_require__(5064);
-      const _routercontextsharedruntime = __webpack_require__(3948);
+      const _getimgprops = __webpack_require__(6985);
+      const _imageconfig = __webpack_require__(8151);
+      const _imageconfigcontextsharedruntime = __webpack_require__(8836);
+      const _warnonce = __webpack_require__(9138);
+      const _routercontextsharedruntime = __webpack_require__(4654);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5825)
+        __webpack_require__(2946)
       );
-      const _usemergedref = __webpack_require__(6956);
+      const _usemergedref = __webpack_require__(5658);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -371,7 +371,7 @@
       /***/
     },
 
-    /***/ 6956: /***/ (module, exports, __webpack_require__) => {
+    /***/ 5658: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -432,7 +432,7 @@
       /***/
     },
 
-    /***/ 2990: /***/ (
+    /***/ 6985: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -448,9 +448,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(5064);
-      const _imageblursvg = __webpack_require__(2243);
-      const _imageconfig = __webpack_require__(876);
+      const _warnonce = __webpack_require__(9138);
+      const _imageblursvg = __webpack_require__(4901);
+      const _imageconfig = __webpack_require__(8151);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -823,7 +823,7 @@
       /***/
     },
 
-    /***/ 2243: /***/ (__unused_webpack_module, exports) => {
+    /***/ 4901: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -878,7 +878,7 @@
       /***/
     },
 
-    /***/ 2470: /***/ (
+    /***/ 5731: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -905,10 +905,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(9608);
-      const _getimgprops = __webpack_require__(2990);
-      const _imagecomponent = __webpack_require__(6730);
+      const _getimgprops = __webpack_require__(6985);
+      const _imagecomponent = __webpack_require__(9569);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(5825)
+        __webpack_require__(2946)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -940,7 +940,7 @@
       /***/
     },
 
-    /***/ 5825: /***/ (__unused_webpack_module, exports) => {
+    /***/ 2946: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -975,7 +975,7 @@
       /***/
     },
 
-    /***/ 2563: /***/ (
+    /***/ 5412: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -992,8 +992,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@19.0.0-rc-cd22717c-20241013/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(7125);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-cd22717c-20241013_re_a5fwrzcgiqxq7kbo7kpyzjc5ji/node_modules/next/image.js
-      var next_image = __webpack_require__(9983);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-cd22717c-20241013_re_zq6nvqen5pqfmm5xeouxrlc4aq/node_modules/next/image.js
+      var next_image = __webpack_require__(9231);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // CONCATENATED MODULE: ./pages/nextjs.png
       /* harmony default export */ const nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -1023,12 +1023,12 @@
       /***/
     },
 
-    /***/ 9983: /***/ (
+    /***/ 9231: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(2470);
+      module.exports = __webpack_require__(5731);
 
       /***/
     },
@@ -1038,7 +1038,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(6441)
+      __webpack_exec__(7486)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for link-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [4644],
   {
-    /***/ 6707: /***/ (
+    /***/ 9202: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/link",
         function () {
-          return __webpack_require__(940);
+          return __webpack_require__(2971);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 2387: /***/ (module, exports) => {
+    /***/ 8382: /***/ (module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -96,7 +96,7 @@
       /***/
     },
 
-    /***/ 3025: /***/ (module, exports, __webpack_require__) => {
+    /***/ 1205: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -108,7 +108,7 @@
           return getDomainLocale;
         },
       });
-      const _normalizetrailingslash = __webpack_require__(5747);
+      const _normalizetrailingslash = __webpack_require__(3250);
       const basePath =
         /* unused pure expression or super */ null && (false || "");
       function getDomainLocale(path, locale, locales, domainLocales) {
@@ -132,7 +132,7 @@
       /***/
     },
 
-    /***/ 2051: /***/ (module, exports, __webpack_require__) => {
+    /***/ 3836: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -149,18 +149,18 @@
       const _react = /*#__PURE__*/ _interop_require_default._(
         __webpack_require__(1299)
       );
-      const _resolvehref = __webpack_require__(3994);
-      const _islocalurl = __webpack_require__(9486);
-      const _formaturl = __webpack_require__(3261);
-      const _utils = __webpack_require__(2625);
-      const _addlocale = __webpack_require__(4546);
-      const _routercontextsharedruntime = __webpack_require__(3948);
-      const _approutercontextsharedruntime = __webpack_require__(9855);
-      const _useintersection = __webpack_require__(2280);
-      const _getdomainlocale = __webpack_require__(3025);
-      const _addbasepath = __webpack_require__(5594);
-      const _routerreducertypes = __webpack_require__(2387);
-      const _usemergedref = __webpack_require__(6956);
+      const _resolvehref = __webpack_require__(8216);
+      const _islocalurl = __webpack_require__(1998);
+      const _formaturl = __webpack_require__(6261);
+      const _utils = __webpack_require__(5528);
+      const _addlocale = __webpack_require__(3774);
+      const _routercontextsharedruntime = __webpack_require__(4654);
+      const _approutercontextsharedruntime = __webpack_require__(92);
+      const _useintersection = __webpack_require__(4930);
+      const _getdomainlocale = __webpack_require__(1205);
+      const _addbasepath = __webpack_require__(7254);
+      const _routerreducertypes = __webpack_require__(8382);
+      const _usemergedref = __webpack_require__(5658);
       const prefetched = new Set();
       function prefetch(router, href, as, options, appOptions, isAppRouter) {
         if (false) {
@@ -588,7 +588,7 @@
       /***/
     },
 
-    /***/ 2280: /***/ (module, exports, __webpack_require__) => {
+    /***/ 4930: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -601,7 +601,7 @@
         },
       });
       const _react = __webpack_require__(1299);
-      const _requestidlecallback = __webpack_require__(8162);
+      const _requestidlecallback = __webpack_require__(7242);
       const hasIntersectionObserver =
         typeof IntersectionObserver === "function";
       const observers = new Map();
@@ -714,7 +714,7 @@
       /***/
     },
 
-    /***/ 6956: /***/ (module, exports, __webpack_require__) => {
+    /***/ 5658: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -775,7 +775,7 @@
       /***/
     },
 
-    /***/ 940: /***/ (
+    /***/ 2971: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -790,7 +790,7 @@
       /* harmony import */ var react_jsx_runtime__WEBPACK_IMPORTED_MODULE_0__ =
         __webpack_require__(7125);
       /* harmony import */ var next_link__WEBPACK_IMPORTED_MODULE_1__ =
-        __webpack_require__(20);
+        __webpack_require__(2778);
       /* harmony import */ var next_link__WEBPACK_IMPORTED_MODULE_1___default =
         /*#__PURE__*/ __webpack_require__.n(
           next_link__WEBPACK_IMPORTED_MODULE_1__
@@ -821,8 +821,12 @@
       /***/
     },
 
-    /***/ 20: /***/ (module, __unused_webpack_exports, __webpack_require__) => {
-      module.exports = __webpack_require__(2051);
+    /***/ 2778: /***/ (
+      module,
+      __unused_webpack_exports,
+      __webpack_require__
+    ) => {
+      module.exports = __webpack_require__(3836);
 
       /***/
     },
@@ -832,7 +836,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(6707)
+      __webpack_exec__(9202)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for 3620-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js

Diff too large to display

Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js

Diff too large to display

Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: b158dff

if (result.experimental?.cacheLife) {
const defaultCacheLifeProfile = result.experimental.cacheLife['default']
if (!defaultCacheLifeProfile) {
throw new Error('No default cacheLife profile.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we just use our default one if one isn't provided instead of erroring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the idea is that it should be resolved when normalizing the cache and so it something would've gone wrong while normalizing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the normalization logic to always fill in the blanks.

} else {
if (defaultCacheLifeProfile.stale === undefined) {
const staticStaleTime = result.experimental.staleTimes?.static
defaultCacheLifeProfile.stale = staticStaleTime
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check this for all profiles or just the default one since we fallback automatically?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing that's special about stale is that it has a default as a separate config object.

The thing that's special about "default" is that it is used to seed the initial value of the "use cache" scope and then it can only be lowered from that implicitly. So all three need to exist on the default to define the implicit values.

Copy link
Contributor Author

@sebmarkbage sebmarkbage Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added cacheLife.default.expire defaulting to expireTime for backwards compat.

@@ -251,6 +251,7 @@ async function exportPageImpl(
page,
components.routeModule as AppRouteRouteModule,
input.renderOpts.incrementalCache,
input.renderOpts.cacheLifeProfiles,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can Infinity be serialized across worker bounds or do we need to handle this custom?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as we use the built-in structured clone default, then yes. As long as we don't turn it into JSON manually.

@sebmarkbage sebmarkbage force-pushed the configurablecachelife branch 2 times, most recently from a60fc42 to c151392 Compare October 14, 2024 02:51
@ijjk ijjk added the tests label Oct 14, 2024
@@ -518,9 +533,10 @@ export function cache(kind: string, id: string, fn: any) {

let stream: ReadableStream

const disable = true // TODO: Reenable the cache scope
Copy link
Contributor Author

@sebmarkbage sebmarkbage Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found a problem with the cacheScope. It doesn't propagate the revalidate/tags options when there's a hit because we only save the stream - not the full cache entry. Will fix in a separate PR (tests still pass because the default cache handler is an infinite synchronous cache).

@sebmarkbage sebmarkbage force-pushed the configurablecachelife branch 3 times, most recently from b95e7eb to 697d08e Compare October 14, 2024 16:19
@sebmarkbage sebmarkbage force-pushed the configurablecachelife branch from 697d08e to e10d06c Compare October 14, 2024 17:45
@sebmarkbage sebmarkbage merged commit fc2a7d2 into vercel:canary Oct 14, 2024
85 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants